[PATCH net v2] netfilter: br_netfilter: reject unsafe neighbour devices for DNAT

[PATCH net v2] netfilter: br_netfilter: reject unsafe neighbour devices for DNAT

[Kyle Zeng]

The IPv4 and IPv6 bridge prerouting callbacks classify DNAT traffic using
the logical input device saved in skb->dev. The physical input port can
have a different bridge master when prerouting resumes, for example after
an NFQUEUE verdict. Netfilter reassembly can also combine the first
fragment's bridge information with a different fragment's input device.

br_nf_pre_routing_finish_bridge() replaces skb->dev with the port's
current bridge master, but looks up the neighbour using the existing
route. This can put an skb with skb->dev pointing to bridge B on an
unresolved neighbour of bridge A. Deleting B and resolving the neighbour
on A then causes a use-after-free in __dev_queue_xmit().

Before calling the neighbour output method, check that the neighbour
keeps skb->dev alive. A neighbour holds its device, and a VLAN device
also holds its underlying device, so allow a VLAN whose real device is
the bridge. Release the neighbour and drop the skb on a mismatch.

With bridge-nf-filter-vlan-tagged=1 and bridge-nf-pass-vlan-input-dev=1,
the logical input device can be a VLAN such as br0.100. The earlier
route-device comparison then succeeds with br0.100, but this continuation
changes skb->dev to br0. Comparing dst->dev directly with skb->dev would
drop valid VLAN DNAT traffic.

Doing this in the shared continuation covers both protocol families,
including a queue in the remaining bridge prerouting hooks. Leave the
cached-header path unchanged since it does not enqueue the skb.

The issue was found by an LLM.

Fixes: 2948d2ebbb98 ("[NETFILTER]: bridge: fix double POST_ROUTING invocation")
Cc: stable@vger.kernel.org
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
v1: https://lore.kernel.org/netfilter-devel/20261030042155.73844-1-kylebot@openai.com/
v2:
- Explain the bridge-nf-pass-vlan-input-dev case in the commit message
  and the neighbour-lifetime comment. No functional change from v1.
- Add the Fixes tag for the change to direct neighbour output.

I have a reproducer for this issue and can share via DM

 net/bridge/br_netfilter_hooks.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index e15355ff215a..1f5c7f284e0f 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -307,6 +307,19 @@ int br_nf_pre_routing_finish_bridge(struct net *net, struct sock *sk, struct sk_
 
 			ret = br_handle_frame_finish(net, sk, skb);
 		} else {
+			struct net_device *dev = neigh->dev;
+
+			/* The neighbour queue must keep skb->dev alive.
+			 * With bridge-nf-pass-vlan-input-dev=1, dst->dev may
+			 * be a VLAN device, which holds its underlying bridge.
+			 */
+			if (is_vlan_dev(dev))
+				dev = vlan_dev_real_dev(dev);
+			if (dev != skb->dev) {
+				neigh_release(neigh);
+				goto free_skb;
+			}
+
 			/* the neighbour function below overwrites the complete
 			 * MAC header, so we save the Ethernet source address and
 			 * protocol number.

base-commit: d48723711bae9b8cdc56719c448ba42fef28cbee
-- 
2.55.0.openai.911.g06873a30d2f3