With vlan_filtering enabled, br_allowed_ingress() moves the vlan tag back into the packet when its protocol differs from the bridge's vlan_protocol, an 802.1Q tag on an 802.1ad bridge for instance, and sets skb->protocol to the vlan ethertype. That happens after the NF_BR_PRE_ROUTING hooks.
Bridge conntrack has already defragmented the packet by then, based on the original protocol. In postrouting nf_ct_bridge_refrag() finds a vlan ethertype it cannot fragment, warns and drops the packet: WARNING: net/bridge/netfilter/nf_conntrack_bridge.c:361 at nf_ct_bridge_post Call trace: nf_ct_bridge_post nf_hook_slow br_forward_finish __br_forward deliver_clone br_flood br_handle_frame_finish br_handle_frame The warning is the smaller problem. Every fragmented IPv4 or IPv6 packet that crosses such a bridge is dropped while unfragmented traffic is fine, so it looks like an MTU issue. And anyone who can send fragments into a port of such a bridge can trigger the warning, which takes the machine down with panic_on_warn. Look past the vlan tag in nf_ct_bridge_refrag(): remember it, pull it, fragment as before and push it back onto each fragment in nf_ct_bridge_frag_restore(), together with the outer protocol, so that br_dev_queue_push_xmit() sees the fragments like any other tagged frame. The protocol behind the tag comes from the packet, so check it against the header before fragmenting. Removing the WARN_ON_ONCE() would only hide the problem, the packets would still be dropped. Not tracking these frames in nf_ct_bridge_pre() instead would change what is tracked today for unfragmented traffic, and would have to duplicate the decision that br_allowed_ingress() makes later. The syzbot reproducer is an IPv6 atomic fragment with a frag_max_size of 60, which br_ip6_fragment() drops silently because it is below IPV6_MIN_MTU. It no longer warns either. Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=a8ba738fe2db6b4bb27f Fixes: 3c171f496ef5 ("netfilter: bridge: add connection tracking system") Fixes: 764dd163ac92 ("netfilter: nf_conntrack_bridge: add support for IPv6") Cc: [email protected] Assisted-by: LLM Signed-off-by: Laveesh Bansal <[email protected]> --- include/linux/netfilter_bridge.h | 4 ++ net/bridge/netfilter/nf_conntrack_bridge.c | 43 ++++++++++++++++++++-- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h index 743475ca7e9d..7091e01abe77 100644 --- a/include/linux/netfilter_bridge.h +++ b/include/linux/netfilter_bridge.h @@ -10,6 +10,10 @@ struct nf_bridge_frag_data { bool vlan_present; u16 vlan_tci; __be16 vlan_proto; + bool inband_vlan_present; + u16 inband_vlan_tci; + __be16 inband_vlan_proto; + __be16 inband_vlan_encap_proto; }; #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c index b5444335b86f..e16071eb2df0 100644 --- a/net/bridge/netfilter/nf_conntrack_bridge.c +++ b/net/bridge/netfilter/nf_conntrack_bridge.c @@ -344,13 +344,41 @@ nf_ct_bridge_refrag(struct sk_buff *skb, const struct nf_hook_state *state, const struct nf_bridge_frag_data *data, struct sk_buff *)) { - struct nf_bridge_frag_data data; + struct nf_bridge_frag_data data = {}; + __be16 proto = skb->protocol; if (!BR_INPUT_SKB_CB(skb)->frag_max_size) return NF_ACCEPT; nf_ct_bridge_frag_save(skb, &data); - switch (skb->protocol) { + + /* A vlan filtering bridge moves a vlan tag into the packet if its + * protocol differs from the bridge's vlan_protocol, after prerouting + * defragmented the packet. Look past that tag. + */ + if (eth_type_vlan(proto)) { + const struct vlan_hdr *vhdr; + + if (skb_network_offset(skb) != VLAN_HLEN || + !pskb_may_pull(skb, VLAN_HLEN)) + return NF_DROP; + + vhdr = (const struct vlan_hdr *)skb->data; + data.inband_vlan_present = true; + data.inband_vlan_tci = ntohs(vhdr->h_vlan_TCI); + data.inband_vlan_proto = proto; + data.inband_vlan_encap_proto = vhdr->h_vlan_encapsulated_proto; + + proto = data.inband_vlan_encap_proto; + __skb_pull(skb, VLAN_HLEN); + skb->protocol = proto; + + if ((proto == htons(ETH_P_IP) && ip_hdr(skb)->version != 4) || + (proto == htons(ETH_P_IPV6) && ipv6_hdr(skb)->version != 6)) + return NF_DROP; + } + + switch (proto) { case htons(ETH_P_IP): nf_br_ip_fragment(state->net, state->sk, skb, &data, output); break; @@ -358,7 +386,7 @@ nf_ct_bridge_refrag(struct sk_buff *skb, const struct nf_hook_state *state, nf_br_ip6_fragment(state->net, state->sk, skb, &data, output); break; default: - WARN_ON_ONCE(1); + WARN_ON_ONCE(!data.inband_vlan_present); return NF_DROP; } @@ -371,11 +399,18 @@ static int nf_ct_bridge_frag_restore(struct sk_buff *skb, { int err; - err = skb_cow_head(skb, ETH_HLEN); + err = skb_cow_head(skb, ETH_HLEN + (data->inband_vlan_present ? VLAN_HLEN : 0)); if (err) { kfree_skb(skb); return -ENOMEM; } + if (data->inband_vlan_present) { + struct vlan_hdr *vhdr = skb_push(skb, VLAN_HLEN); + + vhdr->h_vlan_TCI = htons(data->inband_vlan_tci); + vhdr->h_vlan_encapsulated_proto = data->inband_vlan_encap_proto; + skb->protocol = data->inband_vlan_proto; + } if (data->vlan_present) __vlan_hwaccel_put_tag(skb, data->vlan_proto, data->vlan_tci); else if (skb_vlan_tag_present(skb)) -- 2.54.0 (Apple Git-157)
