Fragmented IPv4 and IPv6 packets are dropped on a vlan filtering bridge with bridge conntrack when the protocol of their vlan tag differs from the vlan_protocol of the bridge, an 802.1Q tag on an 802.1ad bridge for instance. Unfragmented traffic is not affected. br_allowed_ingress() moves such a tag back into the packet after the NF_BR_PRE_ROUTING hooks and sets skb->protocol to the vlan ethertype, but bridge conntrack has already defragmented the packet by then. In postrouting nf_ct_bridge_refrag() does not know that ethertype and drops the packet. On CONFIG_DEBUG_NET kernels the drop also warns, which is what syzbot reported. Save the tag with the ethernet header, pull the tag off before fragmenting and push it back onto each fragment in nf_ct_bridge_frag_restore(), as br_netfilter does with its encapsulation header. A ruleset can rewrite the tag, so check the IP version against its ethertype first. Reported-by: syzbot+a8ba738fe2db6b4bb27f@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?extid=a8ba738fe2db6b4bb27f Signed-off-by: Laveesh Bansal --- include/linux/netfilter_bridge.h | 4 +- net/bridge/netfilter/nf_conntrack_bridge.c | 46 ++++++++++++++++++---- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h index 743475ca7e9d..772f6da27237 100644 --- a/include/linux/netfilter_bridge.h +++ b/include/linux/netfilter_bridge.h @@ -3,10 +3,12 @@ #define __LINUX_BRIDGE_NETFILTER_H #include +#include #include struct nf_bridge_frag_data { - char mac[ETH_HLEN]; + char mac[ETH_HLEN + VLAN_HLEN]; + u8 encap_len; bool vlan_present; u16 vlan_tci; __be16 vlan_proto; diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c index 8c83dc05114b..be4a04e9c90e 100644 --- a/net/bridge/netfilter/nf_conntrack_bridge.c +++ b/net/bridge/netfilter/nf_conntrack_bridge.c @@ -326,8 +326,10 @@ static unsigned int nf_ct_bridge_in(void *priv, struct sk_buff *skb, } static void nf_ct_bridge_frag_save(struct sk_buff *skb, - struct nf_bridge_frag_data *data) + struct nf_bridge_frag_data *data, + unsigned int encap_len) { + data->encap_len = encap_len; if (skb_vlan_tag_present(skb)) { data->vlan_present = true; data->vlan_tci = skb->vlan_tci; @@ -335,7 +337,8 @@ static void nf_ct_bridge_frag_save(struct sk_buff *skb, } else { data->vlan_present = false; } - skb_copy_from_linear_data_offset(skb, -ETH_HLEN, data->mac, ETH_HLEN); + skb_copy_from_linear_data_offset(skb, -ETH_HLEN, data->mac, + ETH_HLEN + encap_len); } static unsigned int @@ -345,12 +348,38 @@ nf_ct_bridge_refrag(struct sk_buff *skb, const struct nf_hook_state *state, struct sk_buff *)) { struct nf_bridge_frag_data data; + __be16 proto = skb->protocol; + unsigned int encap_len = 0; 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 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) { + DEBUG_NET_WARN_ON_ONCE(1); + return NF_DROP; + } + + vhdr = (const struct vlan_hdr *)skb->data; + encap_len = VLAN_HLEN; + proto = vhdr->h_vlan_encapsulated_proto; + + /* A ruleset can rewrite the tag, make sure it still matches */ + if ((proto == htons(ETH_P_IP) && ip_hdr(skb)->version != 4) || + (proto == htons(ETH_P_IPV6) && ipv6_hdr(skb)->version != 6)) + return NF_DROP; + } + + nf_ct_bridge_frag_save(skb, &data, encap_len); + __skb_pull(skb, encap_len); + + switch (proto) { case htons(ETH_P_IP): nf_br_ip_fragment(state->net, state->sk, skb, &data, output); break; @@ -358,20 +387,19 @@ 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: - DEBUG_NET_WARN_ON_ONCE(1); + DEBUG_NET_WARN_ON_ONCE(!encap_len); return NF_DROP; } return NF_STOLEN; } -/* Actually only slow path refragmentation needs this. */ static int nf_ct_bridge_frag_restore(struct sk_buff *skb, const struct nf_bridge_frag_data *data) { int err; - err = skb_cow_head(skb, ETH_HLEN); + err = skb_cow_head(skb, ETH_HLEN + data->encap_len); if (err) { kfree_skb(skb); return -ENOMEM; @@ -381,7 +409,9 @@ static int nf_ct_bridge_frag_restore(struct sk_buff *skb, else if (skb_vlan_tag_present(skb)) __vlan_hwaccel_clear_tag(skb); - skb_copy_to_linear_data_offset(skb, -ETH_HLEN, data->mac, ETH_HLEN); + __skb_push(skb, data->encap_len); + skb_copy_to_linear_data_offset(skb, -ETH_HLEN, data->mac, + ETH_HLEN + data->encap_len); skb_reset_mac_header(skb); return 0; -- 2.43.0