[PATCH 1/2] netfilter: bridge: refragment packets with a vlan tag moved by the bridge

[Laveesh Bansal <laveeshb@laveeshbansal.com>]

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: syzbot+a8ba738fe2db6b4bb27f@syzkaller.appspotmail.com
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: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Laveesh Bansal <laveeshb@laveeshbansal.com>
---
 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)