[PATCH net-next 01/15] netfilter: disable br_netfilter in user namespaces

[PATCH net-next 01/15] netfilter: disable br_netfilter in user namespaces

[Pablo Neira Ayuso]

From: Florian Westphal <fw@strlen.de>

br_netfilter has way too many bugs, schedule for removal and disable
the feature in user namespaces.

In particular, we get more and more gadgets to have bridge ports escape
rcu_read_lock() protection, then underlying device gets reconfigured
(macvlan or something else), then reinject into network stack causes
crash because net_device is assumed to still have bridge rx data
associated with it.

br_netfilter is an ancient kludge that emulates ip stack from within the
bridge and then calls into ipv4/ipv6 netfilter hooks to have bridged
frames processed via iptables/ip6tables by setting the 'call-iptables'
sysctls to 1. It is NOT related to ebtables or nftables 'bridge' family
at all.

Note that in addition to call-iptables, sysctls, the bridge also exposes
call-iptables per bridge device via netlink and sysfs
('BROPT_NF_CALL_IPTABLES') toggles.

Those are still available, the brnet structure is scoped to br_netfilter;
exposing it bridge-wide is a big no-no.

This relies on brnf_device_event() no longer registering the base
br_netfilter hooks, so while BROPT_NF_CALL_IPTABLES can be set, it
has no effect because the hooks that evaluate this feature bit
are never registered.

brnf_init_net can no longer depend on CONFIG_SYSCTL, else such builds
never disable support in user namespaces.

Cc: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 Documentation/networking/bridge.rst |  5 +++--
 net/bridge/br_netfilter_hooks.c     | 33 ++++++++++++++++++++---------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/Documentation/networking/bridge.rst b/Documentation/networking/bridge.rst
index c1e6ea52c9e5..dded1a6edd60 100644
--- a/Documentation/networking/bridge.rst
+++ b/Documentation/networking/bridge.rst
@@ -277,8 +277,9 @@ Netfilter
 =========
 
 The bridge netfilter module is a legacy feature that allows to filter bridged
-packets with iptables and ip6tables. Its use is discouraged. Users should
-consider using nftables for packet filtering.
+packets with iptables and ip6tables. This feature is scheduled for removal and
+not available in user namespaces. Consider using nftables for packet filtering
+instead.
 
 The older ebtables tool is more feature-limited compared to nftables, but
 just like nftables it doesn't need this module either to function.
diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index 0a394e5f4391..e15355ff215a 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -54,6 +54,7 @@ static unsigned int brnf_net_id __read_mostly;
 
 struct brnf_net {
 	bool enabled;
+	bool in_userns;
 
 #ifdef CONFIG_SYSCTL
 	struct ctl_table_header *ctl_hdr;
@@ -1100,7 +1101,7 @@ static int brnf_device_event(struct notifier_block *unused, unsigned long event,
 
 	net = dev_net(dev);
 	brnet = net_generic(net, brnf_net_id);
-	if (brnet->enabled)
+	if (brnet->enabled || brnet->in_userns)
 		return NOTIFY_OK;
 
 	ret = nf_register_net_hooks(net, br_nf_ops, ARRAY_SIZE(br_nf_ops));
@@ -1229,9 +1230,11 @@ static inline void br_netfilter_sysctl_default(struct brnf_net *brnf)
 	brnf->filter_pppoe_tagged = 0;
 	brnf->pass_vlan_indev = 0;
 }
+#endif
 
 static int br_netfilter_sysctl_init_net(struct net *net)
 {
+#ifdef CONFIG_SYSCTL
 	struct ctl_table *table = brnf_table;
 	struct brnf_net *brnet;
 
@@ -1259,25 +1262,39 @@ static int br_netfilter_sysctl_init_net(struct net *net)
 
 		return -ENOMEM;
 	}
-
+#endif
 	return 0;
 }
 
 static void br_netfilter_sysctl_exit_net(struct net *net,
 					 struct brnf_net *brnet)
 {
-	const struct ctl_table *table = brnet->ctl_hdr->ctl_table_arg;
+#ifdef CONFIG_SYSCTL
+	struct ctl_table_header *header = brnet->ctl_hdr;
+	const struct ctl_table *table;
 
-	unregister_net_sysctl_table(brnet->ctl_hdr);
+	if (!header)
+		return;
+
+	table = header->ctl_table_arg;
+	unregister_net_sysctl_table(header);
 	if (!net_eq(net, &init_net))
 		kfree(table);
+#endif
 }
 
 static int __net_init brnf_init_net(struct net *net)
 {
+	if (net->user_ns != &init_user_ns) {
+		struct brnf_net *brnet = net_generic(net, brnf_net_id);
+
+		brnet->in_userns = true;
+		pr_warn_once("br_netfilter no longer supported in user namespaces\n");
+		return 0;
+	}
+
 	return br_netfilter_sysctl_init_net(net);
 }
-#endif
 
 static void __net_exit brnf_exit_net(struct net *net)
 {
@@ -1289,15 +1306,11 @@ static void __net_exit brnf_exit_net(struct net *net)
 		brnet->enabled = false;
 	}
 
-#ifdef CONFIG_SYSCTL
 	br_netfilter_sysctl_exit_net(net, brnet);
-#endif
 }
 
 static struct pernet_operations brnf_net_ops __read_mostly = {
-#ifdef CONFIG_SYSCTL
 	.init = brnf_init_net,
-#endif
 	.exit = brnf_exit_net,
 	.id   = &brnf_net_id,
 	.size = sizeof(struct brnf_net),
@@ -1318,7 +1331,7 @@ static int __init br_netfilter_init(void)
 	}
 
 	RCU_INIT_POINTER(nf_br_ops, &br_ops);
-	printk(KERN_NOTICE "Bridge firewalling registered\n");
+	pr_warn("br_netfilter is slated for removal in 2027.\n");
 	return 0;
 }
 
-- 
2.47.3

[PATCH net-next 00/15] Netfilter updates for net-next

[Pablo Neira Ayuso]

Hi,

The following patchset contains Netfilter/IPVS updates for net-next:

1) Disable br_netfilter in userns and schedule it for removal in 2027,
   from Florian Westphal.

2) Update selftest nft_queue.sh to make it more stable on debug
   kernels, from Jakub Kicinski.

3) Fix a overflow in IPVS sync messages which can only happen in very
   old 2.6 kernels, from Julian Anastasov.

4) Finer grain packet drop in nfnetlink_queue when dependency objects
   are going away, from Antonio Ojea.

5) Add a selftest for the nfnetlink_queue packet drop case, also
   from Antonio Ojea.

6) Flush conntrack gc worker when existing netns to address a unlikely
   UaF, from Florian Westphal.

7) Switch nf_tables chain types to use rcu, otherwise module removal
   can trigger UaF, also from Florian.

8) Remove redundant parsing in nft_exthdr for the tcp options case,
   from Fernando F. Mancera.

9) Remove redundant skb_header_pointer() calls in nft_exthdr for tcp
   options, from Fernando F. Mancera.

10) Fix conntrack_vrf.sh selftest return value in zone subtest,
    from Florian Westphal.

11) Restrict raw table in nft_compat to specific family, hook and
    priority, from Florian Westphal.

Prepare the flowtable to support for IPv4 over IPv6 tunneling and
vice-versa, this includes 4 patches from Lorenzo Bianconi.

12) Explicitly set inner protocol when popping header, this is not
    needed for the existing IPIP and IP6IP6 case because skb->protocol
    remains the same.

13) Simplify tuple tunnel information fill up when parsing outer
    tunnel IP headers.
    
14) Add a new encap_proto which determines the outer tunnel protocol
    family.

15) Prepare flowtable forwarding path to deal with mixed IPv4-IPv6
    tunnels.

Please, pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git nf-next-26-10-18

Thanks.

----------------------------------------------------------------

The following changes since commit a5e7d8e446af9803e37a3b6a4d416fb41178348f:

  Merge branch '100GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue (2026-10-05 17:51:32 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git nf-next-26-10-18

for you to fetch changes up to d0107f38162279d9cb0e012210731176265f835d:

  netfilter: flowtable: unify tunnel push for IPv4 and IPv6 (2026-10-18 02:42:04 +0200)

----------------------------------------------------------------
netfilter pull request 26-10-18

----------------------------------------------------------------
Antonio Ojea (2):
      netfilter: nf_queue: limit the hook drop flush to the unregistered hook point
      selftests: netfilter: nft_queue: check the scope of the hook drop flush

Fernando Fernandez Mancera (2):
      netfilter: nft_exthdr: remove redundant op parsing in tcp_set_init
      netfilter: nft_exthdr: avoid 60 bytes stack buffer in TCP option mangling

Florian Westphal (5):
      netfilter: disable br_netfilter in user namespaces
      netfilter: conntrack: fix unlikely UaF in gc worker
      netfilter: nf_tables: switch chain_types array to rcu
      selftests: netfilter: fix return value in zone subtest
      netfilter: nft_compat: restrict raw table targets/matches

Jakub Kicinski (1):
      selftests: netfilter: nft_queue.sh: only queue icmp echo request/reply

Julian Anastasov (1):
      ipvs: fix buffer overflow when sending sync messages

Lorenzo Bianconi (4):
      netfilter: flowtable: set inner protocol when popping tunnel header
      netfilter: flowtable: populate tunnel tuple regardless of inner protocol
      netfilter: flowtable: add encap_proto to flow_offload_tunnel
      netfilter: flowtable: unify tunnel push for IPv4 and IPv6

 Documentation/networking/bridge.rst                |   5 +-
 include/linux/netdevice.h                          |   1 +
 include/net/netfilter/nf_flow_table.h              |   1 +
 include/net/netfilter/nf_queue.h                   |   3 +-
 net/bridge/br_netfilter_hooks.c                    |  33 ++--
 net/ipv4/ipip.c                                    |   1 +
 net/ipv6/ip6_tunnel.c                              |   1 +
 net/netfilter/core.c                               |   2 +-
 net/netfilter/ipvs/ip_vs_sync.c                    |  23 +--
 net/netfilter/nf_conntrack_core.c                  |   7 +-
 net/netfilter/nf_flow_table_ip.c                   | 170 +++++++++++++--------
 net/netfilter/nf_flow_table_path.c                 |   2 +
 net/netfilter/nf_internals.h                       |   2 +-
 net/netfilter/nf_queue.c                           |   4 +-
 net/netfilter/nf_tables_api.c                      |  94 ++++++++----
 net/netfilter/nfnetlink_queue.c                    |  41 ++++-
 net/netfilter/nft_chain_nat.c                      |   1 +
 net/netfilter/nft_compat.c                         |  15 ++
 net/netfilter/nft_ct.c                             |   2 +-
 net/netfilter/nft_exthdr.c                         |  34 +++--
 .../selftests/net/netfilter/conntrack_vrf.sh       |  25 ++-
 tools/testing/selftests/net/netfilter/nf_queue.c   |  22 ++-
 tools/testing/selftests/net/netfilter/nft_queue.sh | 110 ++++++++++++-
 23 files changed, 456 insertions(+), 143 deletions(-)

[PATCH net-next 01/15] netfilter: disable br_netfilter in user namespaces

[Pablo Neira Ayuso]

From: Florian Westphal <fw@strlen.de>

br_netfilter has way too many bugs, schedule for removal and disable
the feature in user namespaces.

In particular, we get more and more gadgets to have bridge ports escape
rcu_read_lock() protection, then underlying device gets reconfigured
(macvlan or something else), then reinject into network stack causes
crash because net_device is assumed to still have bridge rx data
associated with it.

br_netfilter is an ancient kludge that emulates ip stack from within the
bridge and then calls into ipv4/ipv6 netfilter hooks to have bridged
frames processed via iptables/ip6tables by setting the 'call-iptables'
sysctls to 1. It is NOT related to ebtables or nftables 'bridge' family
at all.

Note that in addition to call-iptables, sysctls, the bridge also exposes
call-iptables per bridge device via netlink and sysfs
('BROPT_NF_CALL_IPTABLES') toggles.

Those are still available, the brnet structure is scoped to br_netfilter;
exposing it bridge-wide is a big no-no.

This relies on brnf_device_event() no longer registering the base
br_netfilter hooks, so while BROPT_NF_CALL_IPTABLES can be set, it
has no effect because the hooks that evaluate this feature bit
are never registered.

brnf_init_net can no longer depend on CONFIG_SYSCTL, else such builds
never disable support in user namespaces.

Cc: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 Documentation/networking/bridge.rst |  5 +++--
 net/bridge/br_netfilter_hooks.c     | 33 ++++++++++++++++++++---------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/Documentation/networking/bridge.rst b/Documentation/networking/bridge.rst
index c1e6ea52c9e5..dded1a6edd60 100644
--- a/Documentation/networking/bridge.rst
+++ b/Documentation/networking/bridge.rst
@@ -277,8 +277,9 @@ Netfilter
 =========
 
 The bridge netfilter module is a legacy feature that allows to filter bridged
-packets with iptables and ip6tables. Its use is discouraged. Users should
-consider using nftables for packet filtering.
+packets with iptables and ip6tables. This feature is scheduled for removal and
+not available in user namespaces. Consider using nftables for packet filtering
+instead.
 
 The older ebtables tool is more feature-limited compared to nftables, but
 just like nftables it doesn't need this module either to function.
diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index 0a394e5f4391..e15355ff215a 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -54,6 +54,7 @@ static unsigned int brnf_net_id __read_mostly;
 
 struct brnf_net {
 	bool enabled;
+	bool in_userns;
 
 #ifdef CONFIG_SYSCTL
 	struct ctl_table_header *ctl_hdr;
@@ -1100,7 +1101,7 @@ static int brnf_device_event(struct notifier_block *unused, unsigned long event,
 
 	net = dev_net(dev);
 	brnet = net_generic(net, brnf_net_id);
-	if (brnet->enabled)
+	if (brnet->enabled || brnet->in_userns)
 		return NOTIFY_OK;
 
 	ret = nf_register_net_hooks(net, br_nf_ops, ARRAY_SIZE(br_nf_ops));
@@ -1229,9 +1230,11 @@ static inline void br_netfilter_sysctl_default(struct brnf_net *brnf)
 	brnf->filter_pppoe_tagged = 0;
 	brnf->pass_vlan_indev = 0;
 }
+#endif
 
 static int br_netfilter_sysctl_init_net(struct net *net)
 {
+#ifdef CONFIG_SYSCTL
 	struct ctl_table *table = brnf_table;
 	struct brnf_net *brnet;
 
@@ -1259,25 +1262,39 @@ static int br_netfilter_sysctl_init_net(struct net *net)
 
 		return -ENOMEM;
 	}
-
+#endif
 	return 0;
 }
 
 static void br_netfilter_sysctl_exit_net(struct net *net,
 					 struct brnf_net *brnet)
 {
-	const struct ctl_table *table = brnet->ctl_hdr->ctl_table_arg;
+#ifdef CONFIG_SYSCTL
+	struct ctl_table_header *header = brnet->ctl_hdr;
+	const struct ctl_table *table;
 
-	unregister_net_sysctl_table(brnet->ctl_hdr);
+	if (!header)
+		return;
+
+	table = header->ctl_table_arg;
+	unregister_net_sysctl_table(header);
 	if (!net_eq(net, &init_net))
 		kfree(table);
+#endif
 }
 
 static int __net_init brnf_init_net(struct net *net)
 {
+	if (net->user_ns != &init_user_ns) {
+		struct brnf_net *brnet = net_generic(net, brnf_net_id);
+
+		brnet->in_userns = true;
+		pr_warn_once("br_netfilter no longer supported in user namespaces\n");
+		return 0;
+	}
+
 	return br_netfilter_sysctl_init_net(net);
 }
-#endif
 
 static void __net_exit brnf_exit_net(struct net *net)
 {
@@ -1289,15 +1306,11 @@ static void __net_exit brnf_exit_net(struct net *net)
 		brnet->enabled = false;
 	}
 
-#ifdef CONFIG_SYSCTL
 	br_netfilter_sysctl_exit_net(net, brnet);
-#endif
 }
 
 static struct pernet_operations brnf_net_ops __read_mostly = {
-#ifdef CONFIG_SYSCTL
 	.init = brnf_init_net,
-#endif
 	.exit = brnf_exit_net,
 	.id   = &brnf_net_id,
 	.size = sizeof(struct brnf_net),
@@ -1318,7 +1331,7 @@ static int __init br_netfilter_init(void)
 	}
 
 	RCU_INIT_POINTER(nf_br_ops, &br_ops);
-	printk(KERN_NOTICE "Bridge firewalling registered\n");
+	pr_warn("br_netfilter is slated for removal in 2027.\n");
 	return 0;
 }
 
-- 
2.47.3

[PATCH net-next 02/15] selftests: netfilter: nft_queue.sh: only queue icmp echo request/reply

[Pablo Neira Ayuso]

From: Jakub Kicinski <kuba@kernel.org>

The bridge test flakes on debug kernels:

  FAIL: Expected 10 packets total, but got 24 packets total
  hook 3 packets 00000008
  hook 4 packets 00000010

The surplus are icmp fragment reassembly timeouts.  The udp flood in the
stress test leaves incomplete datagrams behind in ns2 and ns3, 30 seconds
later their reassembly queues expire and both namespaces send icmp time
exceeded to ns1's pre-bridge address.  ns3 is not reconfigured when the
router is turned into a bridge, so its messages arrive via veth2 and are
then forwarded out of br0. Such packets are locally originated from the
bridge point of view and are queued from the bridge output and
postrouting hooks, which is why only those two counters are off.

Restrict the ipv4 rule to echo request/reply, the icmpv6 rule already
does this.

We used to see 1 flake a day in NIPA before locally queuing this change,
zero flakes since (over 9 days)

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 tools/testing/selftests/net/netfilter/nft_queue.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/netfilter/nft_queue.sh b/tools/testing/selftests/net/netfilter/nft_queue.sh
index 7c857a2e0f34..c8d1f2eb4133 100755
--- a/tools/testing/selftests/net/netfilter/nft_queue.sh
+++ b/tools/testing/selftests/net/netfilter/nft_queue.sh
@@ -92,7 +92,7 @@ load_ruleset() {
 ip netns exec "$nsrouter" nft -f /dev/stdin <<EOF
 table $family $name {
 	chain nfq {
-		ip protocol icmp queue bypass
+		icmp type { "echo-request", "echo-reply" } queue bypass
 		icmpv6 type { "echo-request", "echo-reply" } queue num 1 bypass
 	}
 	chain pre {
-- 
2.47.3

[PATCH net-next 03/15] ipvs: fix buffer overflow when sending sync messages

[Pablo Neira Ayuso]

From: Julian Anastasov <ja@ssi.bg>

Sashiko reports for possible heap buffer overflow when
generating sync message for the v0 and v1 message formats.
We should read the cp->flags only once because another
CPU can concurrently set the SEQ_MASK between the two
reads.

Note that IPVS does not set the SEQ_MASK anymore for the
ip_vs_ftp.c helper starting from commit 7f1c40757951
("IPVS: make FTP work with full NAT support") (2.6.36+),
so the problem should not be reproducible.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Link: https://sashiko.dev/#/patchset/20260903004149.1037028-1-pablo%40netfilter.org
Link: https://sashiko.dev/#/patchset/20260909111338.44357-1-ja%40ssi.bg
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipvs/ip_vs_sync.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 45655cff078c..157f5c8361ab 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -543,13 +543,15 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp,
 	struct ip_vs_sync_conn_v0 *s;
 	struct ip_vs_sync_buff *buff;
 	struct ipvs_master_sync_state *ms;
-	int id;
+	u32 flags, seq_mask;
 	unsigned int len;
+	int id;
 
 	if (unlikely(cp->af != AF_INET))
 		return;
+	flags = READ_ONCE(cp->flags);
 	/* Do not sync ONE PACKET */
-	if (cp->flags & IP_VS_CONN_F_ONE_PACKET)
+	if (flags & IP_VS_CONN_F_ONE_PACKET)
 		return;
 
 	if (!ip_vs_sync_conn_needed(ipvs, cp, pkts))
@@ -564,8 +566,8 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp,
 	id = select_master_thread_id(ipvs, cp);
 	ms = &ipvs->ms[id];
 	buff = ms->sync_buff;
-	len = (cp->flags & IP_VS_CONN_F_SEQ_MASK) ? FULL_CONN_SIZE :
-		SIMPLE_CONN_SIZE;
+	seq_mask = flags & IP_VS_CONN_F_SEQ_MASK;
+	len = seq_mask ? FULL_CONN_SIZE : SIMPLE_CONN_SIZE;
 	if (buff) {
 		m = (struct ip_vs_sync_mesg_v0 *) buff->mesg;
 		/* Send buffer if it is for v1 */
@@ -597,9 +599,9 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp,
 	s->caddr = cp->caddr.ip;
 	s->vaddr = cp->vaddr.ip;
 	s->daddr = cp->daddr.ip;
-	s->flags = htons(cp->flags & ~IP_VS_CONN_F_HASHED);
+	s->flags = htons(flags & ~IP_VS_CONN_F_HASHED);
 	s->state = htons(cp->state);
-	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
+	if (seq_mask) {
 		struct ip_vs_sync_conn_options *opt =
 			(struct ip_vs_sync_conn_options *)&s[1];
 		memcpy(opt, &cp->sync_conn_opt, sizeof(*opt));
@@ -635,6 +637,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
 	int id;
 	__u8 *p;
 	unsigned int len, pe_name_len, pad;
+	u32 flags, seq_mask;
 
 	/* Handle old version of the protocol */
 	if (sysctl_sync_ver(ipvs) == 0) {
@@ -647,6 +650,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
 sloop:
 	if (!ip_vs_sync_conn_needed(ipvs, cp, pkts))
 		goto control;
+	flags = READ_ONCE(cp->flags);
 
 	/* Sanity checks */
 	pe_name_len = 0;
@@ -674,7 +678,8 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
 #endif
 		len = sizeof(struct ip_vs_sync_v4);
 
-	if (cp->flags & IP_VS_CONN_F_SEQ_MASK)
+	seq_mask = flags & IP_VS_CONN_F_SEQ_MASK;
+	if (seq_mask)
 		len += sizeof(struct ip_vs_sync_conn_options) + 2;
 
 	if (cp->pe_data_len)
@@ -720,7 +725,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
 	/* Set message type  & copy members */
 	s->v4.type = (cp->af == AF_INET6 ? STYPE_F_INET6 : 0);
 	s->v4.ver_size = htons(len & SVER_MASK);	/* Version 0 */
-	s->v4.flags = htonl(cp->flags & ~IP_VS_CONN_F_HASHED);
+	s->v4.flags = htonl(flags & ~IP_VS_CONN_F_HASHED);
 	s->v4.state = htons(cp->state);
 	s->v4.protocol = cp->protocol;
 	s->v4.cport = cp->cport;
@@ -744,7 +749,7 @@ void ip_vs_sync_conn(struct netns_ipvs *ipvs, struct ip_vs_conn *cp, int pkts)
 		s->v4.vaddr = cp->vaddr.ip;
 		s->v4.daddr = cp->daddr.ip;
 	}
-	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
+	if (seq_mask) {
 		*(p++) = IPVS_OPT_SEQ_DATA;
 		*(p++) = sizeof(struct ip_vs_sync_conn_options);
 		hton_seq(&cp->in_seq, (struct ip_vs_seq *)p);
-- 
2.47.3

[PATCH net-next 04/15] netfilter: nf_queue: limit the hook drop flush to the unregistered hook point

[Pablo Neira Ayuso]

From: Antonio Ojea <aojea@google.com>

Unregistering a netfilter hook calls nf_queue_nf_hook_drop(), which makes
nfqnl_nf_hook_drop() walk every nfqueue instance of the network namespace
and flush it, reinjecting every packet that waits for a verdict with
NF_DROP. Deleting an nftables base chain of any table, family and hook
point therefore drops the packets queued by hooks of unrelated tables,
families and hook points. The verdict that userspace sends for them
afterwards fails with -ENOENT.

Before commit 039b40ee5854 ("netfilter: nf_queue: only call
synchronize_net twice if nf_queue is active") nf_queue_nf_hook_drop()
received the removed hook and nfqueue only dropped the entries queued by
that hook. The commit removed the argument and the comparison because
base chain removal while nfqueue is in use was rare at the time. With
base chains added and removed by userspace agents while another program
holds packets in a queue this is no longer the case. Commit
960632ece694 ("netfilter: convert hook list to an array") then replaced
the hook pointer in struct nf_queue_entry with an index into the hook
array of the hook point, so a comparison per hook is no longer possible.
A comparison per hook point is: the index of an entry is only stale when
the array of its own hook point changed.

Pass the nf_hook_ops being unregistered down to the queue handler and
add nf_hook_cmp() to nfnetlink_queue, a cmpfn for nfqnl_flush() that
matches the entries whose state.pf and state.hook are the hook point of
that hook. The handler takes the nf_hook_ops rather than the family of
the modified array to keep a single argument with NULL meaning "flush
everything", which nf_ct_iterate_destroy() uses on module exit and
__nft_ct_set_destroy() uses when a ct zone expression goes away and a
queued packet may still reference its conntrack template. The
cmpfn resolves the family itself. An NFPROTO_INET hook is registered in
the IPv4 and the IPv6 array, so it matches the entries of both families;
nf_unregister_net_hook() calls __nf_unregister_net_hook() for the two
arrays back to back and the second flush finds nothing left. If the
shrink of one array fails to allocate, that array keeps its dummy hook
and its entries stay valid, so the flush for the other family drops
them without need; no entry with a stale index survives. An
NFPROTO_INET hook at NF_INET_INGRESS is registered in the netdev ingress
array, the same mapping that nf_static_key_inc() applies. nfqueue does
not accept the ingress and egress hooks (nft_queue_validate()), the
mapping keeps the comparison consistent with the registration.

The entries queued from the hook point whose array changed are still
dropped, their hook_index is stale. Registering a hook in front of the
one that queued a packet still makes nf_reinject() resume at a stale
index, so the packet traverses the queueing hook a second time; this is
not changed here.

Tested with the reproducer script from the thread, unchanged: it queues
20 UDP packets from an inet output chain to a queue program that delays
every verdict by 100 ms and changes the ruleset while they wait. Before
this change deleting an unrelated ip6 base chain or an unrelated ip6
table delivers 3 of 20 packets and the queue program exits on ENOENT;
with it all 20 are delivered. Deleting a base chain at the same hook
point (inet output, another priority, in the same or another table) or
an ip table with an output base chain still drops the queued packets;
deleting an ip6 table with an output base chain or a netdev ingress
chain does not. Adding a base chain in front of the queueing one still
queues the packets twice. The nft_queue.sh selftest passes, with
CONFIG_NETFILTER_NETLINK_QUEUE=y and built as a module. The following
patch adds a test for both outcomes that holds the verdicts instead of
delaying them.

The code and this commit message were written with an LLM coding
assistant from a task description that contained the report, the
suggestion from the thread and the two commits above. The author
reviewed the result, directed the changes and validated them with the
reproducer and the selftest in virtme-ng before and after the change.

Suggested-by: Florian Westphal <fw@strlen.de>
Link: https://lore.kernel.org/netfilter-devel/CABhP=tYtCyr-qzL==98nkmDVZr6x+f8E26V1AAOpMXgZgt0vAQ@mail.gmail.com/
Assisted-by: LLM
Signed-off-by: Antonio Ojea <aojea@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_queue.h  |  3 ++-
 net/netfilter/core.c              |  2 +-
 net/netfilter/nf_conntrack_core.c |  2 +-
 net/netfilter/nf_internals.h      |  2 +-
 net/netfilter/nf_queue.c          |  4 +--
 net/netfilter/nfnetlink_queue.c   | 41 +++++++++++++++++++++++++++++--
 net/netfilter/nft_ct.c            |  2 +-
 7 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/include/net/netfilter/nf_queue.h b/include/net/netfilter/nf_queue.h
index fc3e81c07364..86021c55ba3c 100644
--- a/include/net/netfilter/nf_queue.h
+++ b/include/net/netfilter/nf_queue.h
@@ -35,7 +35,8 @@ struct nf_queue_entry {
 struct nf_queue_handler {
 	int		(*outfn)(struct nf_queue_entry *entry,
 				 unsigned int queuenum);
-	void		(*nf_hook_drop)(struct net *net);
+	void		(*nf_hook_drop)(struct net *net,
+					const struct nf_hook_ops *ops);
 };
 
 void nf_register_queue_handler(const struct nf_queue_handler *qh);
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 940dea2663e9..44c856c1e621 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -529,7 +529,7 @@ static void __nf_unregister_net_hook(struct net *net, int pf,
 	if (!p)
 		return;
 
-	nf_queue_nf_hook_drop(net);
+	nf_queue_nf_hook_drop(net, reg);
 	nf_hook_entries_free(p);
 }
 
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index f9b8927327ba..8514a37c29fa 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -2410,7 +2410,7 @@ nf_ct_iterate_destroy(int (*iter)(struct nf_conn *i, void *data), void *data)
 
 		if (atomic_read(&cnet->count) == 0)
 			continue;
-		nf_queue_nf_hook_drop(net);
+		nf_queue_nf_hook_drop(net, NULL);
 	}
 	up_read(&net_rwsem);
 
diff --git a/net/netfilter/nf_internals.h b/net/netfilter/nf_internals.h
index 25403023060b..ce5aff2cae71 100644
--- a/net/netfilter/nf_internals.h
+++ b/net/netfilter/nf_internals.h
@@ -24,7 +24,7 @@
 #define CTA_FILTER_FLAG(ctattr) CTA_FILTER_F_ ## ctattr
 
 /* nf_queue.c */
-void nf_queue_nf_hook_drop(struct net *net);
+void nf_queue_nf_hook_drop(struct net *net, const struct nf_hook_ops *ops);
 
 /* nf_log.c */
 int __init netfilter_log_init(void);
diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index 73363ceedebe..166d05e994ac 100644
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -129,14 +129,14 @@ bool nf_queue_entry_get_refs(struct nf_queue_entry *entry)
 }
 EXPORT_SYMBOL_GPL(nf_queue_entry_get_refs);
 
-void nf_queue_nf_hook_drop(struct net *net)
+void nf_queue_nf_hook_drop(struct net *net, const struct nf_hook_ops *ops)
 {
 	const struct nf_queue_handler *qh;
 
 	rcu_read_lock();
 	qh = rcu_dereference(nf_queue_handler);
 	if (qh)
-		qh->nf_hook_drop(net);
+		qh->nf_hook_drop(net, ops);
 	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(nf_queue_nf_hook_drop);
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index a3bc00280051..00b76c25d55c 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -1559,9 +1559,39 @@ static struct notifier_block nfqnl_dev_notifier = {
 	.notifier_call	= nfqnl_rcv_dev_event,
 };
 
-static void nfqnl_nf_hook_drop(struct net *net)
+/* Match the entries whose hook_index points into the hook array that
+ * __nf_unregister_net_hook() replaces when it removes @ops.
+ *
+ * ops->pf/hooknum name the hook as registered; entry->state.pf/hook
+ * name the array actually traversed - they differ only for NFPROTO_INET.
+ * Every other family uses the array of its own name.
+ */
+static int nf_hook_cmp(struct nf_queue_entry *entry, unsigned long ops_ptr)
+{
+	const struct nf_hook_ops *ops = (const struct nf_hook_ops *)ops_ptr;
+	unsigned int hooknum = ops->hooknum;
+	u8 pf = ops->pf;
+
+	/* same translation as nf_static_key_inc() */
+	if (pf == NFPROTO_INET && hooknum == NF_INET_INGRESS) {
+		pf = NFPROTO_NETDEV;
+		hooknum = NF_NETDEV_INGRESS;
+	}
+
+	if (entry->state.hook != hooknum)
+		return 0;
+
+	if (pf == NFPROTO_INET)
+		return entry->state.pf == NFPROTO_IPV4 ||
+		       entry->state.pf == NFPROTO_IPV6;
+
+	return entry->state.pf == pf;
+}
+
+static void nfqnl_nf_hook_drop(struct net *net, const struct nf_hook_ops *ops)
 {
 	struct nfnl_queue_net *q = nfnl_queue_pernet(net);
+	nfqnl_cmpfn cmpfn = NULL;
 	int i;
 
 	/* This function is also called on net namespace error unwind,
@@ -1574,12 +1604,19 @@ static void nfqnl_nf_hook_drop(struct net *net)
 	if (!q)
 		return;
 
+	/* ops is NULL when the caller is not unregistering a hook but
+	 * removing an object queued packets may reference, every entry
+	 * has to go
+	 */
+	if (ops)
+		cmpfn = nf_hook_cmp;
+
 	for (i = 0; i < INSTANCE_BUCKETS; i++) {
 		struct nfqnl_instance *inst;
 		struct hlist_head *head = &q->instance_table[i];
 
 		hlist_for_each_entry_rcu(inst, head, hlist)
-			nfqnl_flush(inst, NULL, 0);
+			nfqnl_flush(inst, cmpfn, (unsigned long)ops);
 	}
 }
 
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index 3c4c2faa7398..5a8db5592036 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -542,7 +542,7 @@ static void __nft_ct_set_destroy(const struct nft_ctx *ctx, struct nft_ct *priv)
 #endif
 #ifdef CONFIG_NF_CONNTRACK_ZONES
 	case NFT_CT_ZONE:
-		nf_queue_nf_hook_drop(ctx->net);
+		nf_queue_nf_hook_drop(ctx->net, NULL);
 		mutex_lock(&nft_ct_pcpu_mutex);
 		if (--nft_ct_pcpu_template_refcnt == 0)
 			nft_ct_tmpl_put_pcpu();
-- 
2.47.3

[PATCH net-next 05/15] selftests: netfilter: nft_queue: check the scope of the hook drop flush

[Pablo Neira Ayuso]

From: Antonio Ojea <aojea@google.com>

Add test_hook_drop_scope. It queues 20 UDP packets from an inet output
chain to a queue program that holds all verdicts, deletes a base chain
while they wait, and reads the queue length from
/proc/net/netfilter/nfnetlink_queue right after the deletion. It then
releases the queue program and checks with a counter in the input
chain and the packet total of the queue program that the surviving
packets are delivered.

Deleting a base chain at another hook point (ip6 prerouting) must keep
the 20 queued packets. Deleting a base chain at the queueing hook point
(inet output, another priority) drops them, because the index of a
queued packet into the hook array of its hook point is stale once that
array changed. The second case documents the remaining behaviour so
that a change to it is visible.

To hold the packets without depending on timing, add the -H option to
the nf_queue helper: it blocks SIGUSR1, reads the first packet and
waits in sigwait() until it receives SIGUSR1, then sends the verdicts
as usual. The test sends the signal after the ruleset change, so the
queue length it reads is exact on slow machines too.

Without the previous patch the first case fails: 0 of 20 packets remain
queued after the deletion and none is delivered.

The test and the helper option were written with an LLM coding
assistant from a task description; the author reviewed the result,
and validated the test in virtme-ng on kernels with and without the
previous patch.

Assisted-by: LLM
Signed-off-by: Antonio Ojea <aojea@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 .../selftests/net/netfilter/nf_queue.c        |  22 +++-
 .../selftests/net/netfilter/nft_queue.sh      | 108 ++++++++++++++++++
 2 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/netfilter/nf_queue.c b/tools/testing/selftests/net/netfilter/nf_queue.c
index 8bbec37f5356..cca6c3cfa9e4 100644
--- a/tools/testing/selftests/net/netfilter/nf_queue.c
+++ b/tools/testing/selftests/net/netfilter/nf_queue.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <errno.h>
+#include <signal.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdint.h>
@@ -21,6 +22,7 @@ struct options {
 	bool failopen;
 	bool out_of_order;
 	bool bogus_verdict;
+	bool hold;
 	int verbose;
 	unsigned int queue_num;
 	unsigned int timeout;
@@ -34,6 +36,7 @@ static struct options opts;
 static void help(const char *p)
 {
 	printf("Usage: %s [-c|-v [-vv] ] [-o] [-O] [-b] [-t timeout] [-q queue_num] [-Qdst_queue ] [ -d ms_delay ] [-G]\n", p);
+	printf("          [-H] (hold verdicts until SIGUSR1 is received)\n");
 }
 
 static int parse_attr_cb(const struct nlattr *attr, void *data)
@@ -280,6 +283,7 @@ static int mainloop(void)
 	uint32_t ooo_ids[16];
 	unsigned int portid;
 	int ooo_count = 0;
+	sigset_t hold_set;
 	char *buf;
 	int ret;
 
@@ -289,6 +293,12 @@ static int mainloop(void)
 		exit(EXIT_FAILURE);
 	}
 
+	/* -H: keep SIGUSR1 pending until the first packet is read */
+	sigemptyset(&hold_set);
+	sigaddset(&hold_set, SIGUSR1);
+	if (opts.hold)
+		sigprocmask(SIG_BLOCK, &hold_set, NULL);
+
 	nl = open_queue();
 	portid = mnl_socket_get_portid(nl);
 
@@ -320,6 +330,13 @@ static int mainloop(void)
 		}
 
 		id = ret - MNL_CB_OK;
+		if (opts.hold) {
+			int sig;
+
+			sigwait(&hold_set, &sig);
+			opts.hold = false;
+		}
+
 		if (opts.delay_ms)
 			sleep_ms(opts.delay_ms);
 
@@ -364,7 +381,7 @@ static void parse_opts(int argc, char **argv)
 {
 	int c;
 
-	while ((c = getopt(argc, argv, "chvoObt:q:Q:d:G")) != -1) {
+	while ((c = getopt(argc, argv, "chvoObt:q:Q:d:GH")) != -1) {
 		switch (c) {
 		case 'c':
 			opts.count_packets = true;
@@ -373,6 +390,9 @@ static void parse_opts(int argc, char **argv)
 			help(argv[0]);
 			exit(0);
 			break;
+		case 'H':
+			opts.hold = true;
+			break;
 		case 'q':
 			opts.queue_num = atoi(optarg);
 			if (opts.queue_num > 0xffff)
diff --git a/tools/testing/selftests/net/netfilter/nft_queue.sh b/tools/testing/selftests/net/netfilter/nft_queue.sh
index c8d1f2eb4133..a07652813a4b 100755
--- a/tools/testing/selftests/net/netfilter/nft_queue.sh
+++ b/tools/testing/selftests/net/netfilter/nft_queue.sh
@@ -861,6 +861,113 @@ test_queue_bridge()
 	test_queue 20 "bridge"
 }
 
+hook_drop_delivered()
+{
+	ip netns exec "$ns1" nft list counter inet nfqscope delivered |
+		sed -n 's/.*packets \([0-9]*\) .*/\1/p'
+}
+
+hook_drop_queued()
+{
+	ip netns exec "$ns1" awk '$1 == 1 { print $3 }' /proc/self/net/netfilter/nfnetlink_queue
+}
+
+hook_drop_all_queued()
+{
+	[ "$(hook_drop_queued)" = "$1" ]
+}
+
+# hook_drop_case <keep|drop> <description> <nft command...>
+#
+# 1. The inet output chain sends udp packets to port 12345 to queue 1,
+#    the inet input chain counts them once they are accepted.
+# 2. nf_queue -H reads the packets but sends no verdict until it gets
+#    SIGUSR1, so the 20 packets sent stay in the kernel queue.
+# 3. The nft command removes a base chain while they are queued.  The
+#    queue length in /proc right after it tells if the kernel dropped
+#    them: still 20, or 0.
+# 4. SIGUSR1 releases nf_queue.  If the kernel kept the packets they are
+#    accepted now and reach the input counter.  If it dropped them the
+#    first verdict fails with ENOENT and nothing is counted.
+hook_drop_case()
+{
+	local expect="$1"
+	local desc="$2"
+	local packets=20
+	local delivered queued result
+	shift 2
+
+ip netns exec "$ns1" nft -f /dev/stdin <<EOF
+flush ruleset
+table inet nfqscope {
+	chain output {
+		type filter hook output priority 0; policy accept;
+		udp dport 12345 queue num 1
+	}
+	chain input {
+		type filter hook input priority 0; policy accept;
+		udp dport 12345 counter name delivered
+	}
+	chain output2 {
+		type filter hook output priority 100; policy accept;
+	}
+	counter delivered {}
+}
+table ip6 unrelated {
+	chain prerouting {
+		type filter hook prerouting priority 0; policy accept;
+	}
+}
+EOF
+	ip netns exec "$ns1" ./nf_queue -c -q 1 -H -t "$timeout" > "$TMPFILE1" 2>&1 &
+	local nfqpid=$!
+
+	busywait "$BUSYWAIT_TIMEOUT" nf_queue_wait "$ns1" 1
+
+	ip netns exec "$ns1" bash -c \
+		"for i in \$(seq $packets); do echo x > /dev/udp/127.0.0.1/12345; done"
+
+	busywait "$BUSYWAIT_TIMEOUT" hook_drop_all_queued "$packets"
+
+	ip netns exec "$ns1" nft "$@"
+	queued=$(hook_drop_queued)
+
+	kill -USR1 "$nfqpid"
+	wait "$nfqpid"
+	delivered=$(hook_drop_delivered)
+
+	if [ "$queued" = "$packets" ] && [ "$delivered" = "$packets" ] &&
+	   grep -q "^$packets packets total" "$TMPFILE1"; then
+		result="keep"
+	elif [ "$queued" = "0" ] && [ "$delivered" = "0" ]; then
+		result="drop"
+	else
+		result="inconsistent"
+	fi
+
+	if [ "$result" = "$expect" ]; then
+		echo "PASS: $desc: queued packets $result"
+	else
+		echo "FAIL: $desc: queued packets $result, expected $expect" 1>&2
+		echo "      $queued of $packets queued after the change, $delivered delivered" 1>&2
+		ret=1
+	fi
+}
+
+test_hook_drop_scope()
+{
+	# the hook array of another hook point changes, queued packets stay
+	hook_drop_case keep "base chain removed at another hook point" \
+		delete chain ip6 unrelated prerouting
+
+	# the hook array of the queueing hook point changes, the position of
+	# the queued packets in it is stale, they are dropped
+	hook_drop_case drop "base chain removed at the queueing hook point" \
+		delete chain inet nfqscope output2
+
+	ip netns exec "$ns1" nft flush ruleset
+}
+
 ip netns exec "$nsrouter" sysctl net.ipv6.conf.all.forwarding=1 > /dev/null
 ip netns exec "$nsrouter" sysctl net.ipv4.conf.veth0.forwarding=1 > /dev/null
 ip netns exec "$nsrouter" sysctl net.ipv4.conf.veth1.forwarding=1 > /dev/null
@@ -909,6 +1016,7 @@ test_queue_stress
 # should be last, adds vrf device in ns1 and changes routes
 test_icmp_vrf
 test_queue_removal
+test_hook_drop_scope
 
 # turns router into a bridge
 test_queue_bridge
-- 
2.47.3

[PATCH net-next 06/15] netfilter: conntrack: fix unlikely UaF in gc worker

[Pablo Neira Ayuso]

From: Florian Westphal <fw@strlen.de>

When early-drop scan is enabled, gc_worker() reads tmp->ct_net, resolves
cnet with nf_ct_pernet(), and reads cnet->count with RCU read lock held but
before acquiring a reference to tmp.

Its unlikely but possible that the worker gets preempted before reading
cnet->count while nf_conntrack_cleanup_net_list manages to unlink all
conntracks and returns in the mean time.
This allows for the pernet memory to get free'd.

On resume, use after free would occur when the worker reads cnet->count.

This can be solved either via another synchronize_rcu() in
nf_conntrack_cleanup_net_list or via cheaper flush_work().  If work
wasn't running no race can happen as next call cannot observe
already-unlinked conntrack objects.

This is an LLM finding with no reproducer, but it appears to be correct.

Fixes: c6dd940b1f74 ("netfilter: allow early drop of assured conntracks")
Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 8514a37c29fa..931b76aacd93 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -2506,6 +2506,11 @@ void nf_conntrack_cleanup_net_list(struct list_head *net_exit_list)
 		goto i_see_dead_people;
 	}
 
+	/* must wait for current scan loop, worker also looks at
+	 * struct nf_conntrack_net, which is about to be free'd.
+	 */
+	flush_work(&conntrack_gc_work.dwork.work);
+
 	list_for_each_entry(net, net_exit_list, exit_list) {
 		warn_on_keymap_list_leak(net);
 		nf_conntrack_ecache_pernet_fini(net);
-- 
2.47.3

[PATCH net-next 07/15] netfilter: nf_tables: switch chain_types array to rcu

[Pablo Neira Ayuso]

From: Florian Westphal <fw@strlen.de>

LLM quote:
 The global chain_type array is cleared under the nfnetlink subsystem
 mutex, but transaction processing reads it while holding only the pernet
 transaction mutex.
 nf_tables_chain_type_lookup() and nft_chain_parse_hook() dereference
 type->name/hook_mask before try_module_get(type->owner).

 Concurrent unloading of a separate provider such as nft_chain_nat can
 unregister and free its static nft_chain_type while a transaction is
 in the unprotected interval, causing a module-memory use-after-free.

LLM finding, not observed in practice.  Move this under RCU and grab the
module owner earlier.

Both nft_chain_route and nft_chain_filter are built into nf_tables.ko:
only nft_chain_nat needs an extra synchronize_rcu().

Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 94 ++++++++++++++++++++++++-----------
 net/netfilter/nft_chain_nat.c |  1 +
 2 files changed, 65 insertions(+), 30 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index b59628e6240c..59ac623e6805 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1036,32 +1036,67 @@ static inline u64 nf_tables_alloc_handle(struct nft_table *table)
 	return ++table->hgenerator;
 }
 
-static const struct nft_chain_type *chain_type[NFPROTO_NUMPROTO][NFT_CHAIN_T_MAX];
+static const struct nft_chain_type __rcu *chain_type[NFPROTO_NUMPROTO][NFT_CHAIN_T_MAX];
 
 static const struct nft_chain_type *
-__nft_chain_type_get(u8 family, enum nft_chain_types type)
+__nft_chain_type_peek(u8 family, enum nft_chain_types type)
 {
 	if (family >= NFPROTO_NUMPROTO ||
 	    type >= NFT_CHAIN_T_MAX)
 		return NULL;
 
-	return chain_type[family][type];
+	return rcu_dereference_check(chain_type[family][type],
+				     lockdep_nfnl_is_held(NFNL_SUBSYS_NFTABLES));
 }
 
+/**
+ * nft_chain_type_get - look up a chain type by family and id
+ * @family: netlink family
+ * @type:   chain type (enum nft_chain_types)
+ *
+ * Return: chain type with a held module ref, or NULL.
+ */
+static const struct nft_chain_type *
+nft_chain_type_get(u8 family, enum nft_chain_types type)
+{
+	const struct nft_chain_type *t;
+
+	rcu_read_lock();
+	t = __nft_chain_type_peek(family, type);
+
+	if (t && !try_module_get(t->owner))
+		t = NULL;
+
+	rcu_read_unlock();
+	return t;
+}
+
+/**
+ * __nf_tables_chain_type_lookup - look up a chain type by name
+ * @nla:    nlattr containing the chain-type name
+ * @family: netlink family
+ *
+ * Return: matching chain type with a held module ref, or NULL.
+ */
 static const struct nft_chain_type *
 __nf_tables_chain_type_lookup(const struct nlattr *nla, u8 family)
 {
-	const struct nft_chain_type *type;
+	const struct nft_chain_type *type = NULL, *t;
 	int i;
 
+	rcu_read_lock();
 	for (i = 0; i < NFT_CHAIN_T_MAX; i++) {
-		type = __nft_chain_type_get(family, i);
-		if (!type)
+		t = __nft_chain_type_peek(family, i);
+		if (!t)
 			continue;
-		if (!nla_strcmp(nla, type->name))
-			return type;
+
+		if (!nla_strcmp(nla, t->name) && try_module_get(t->owner)) {
+			type = t;
+			break;
+		}
 	}
-	return NULL;
+	rcu_read_unlock();
+	return type;
 }
 
 struct nft_module_request {
@@ -1837,11 +1872,11 @@ static void nf_tables_table_destroy(struct nft_table *table)
 void nft_register_chain_type(const struct nft_chain_type *ctype)
 {
 	nfnl_lock(NFNL_SUBSYS_NFTABLES);
-	if (WARN_ON(__nft_chain_type_get(ctype->family, ctype->type))) {
+	if (WARN_ON(__nft_chain_type_peek(ctype->family, ctype->type))) {
 		nfnl_unlock(NFNL_SUBSYS_NFTABLES);
 		return;
 	}
-	chain_type[ctype->family][ctype->type] = ctype;
+	rcu_assign_pointer(chain_type[ctype->family][ctype->type], ctype);
 	nfnl_unlock(NFNL_SUBSYS_NFTABLES);
 }
 EXPORT_SYMBOL_GPL(nft_register_chain_type);
@@ -1849,7 +1884,7 @@ EXPORT_SYMBOL_GPL(nft_register_chain_type);
 void nft_unregister_chain_type(const struct nft_chain_type *ctype)
 {
 	nfnl_lock(NFNL_SUBSYS_NFTABLES);
-	chain_type[ctype->family][ctype->type] = NULL;
+	RCU_INIT_POINTER(chain_type[ctype->family][ctype->type], NULL);
 	nfnl_unlock(NFNL_SUBSYS_NFTABLES);
 }
 EXPORT_SYMBOL_GPL(nft_unregister_chain_type);
@@ -2581,10 +2616,6 @@ static int nft_chain_parse_hook(struct net *net,
 		hook->num = ntohl(nla_get_be32(ha[NFTA_HOOK_HOOKNUM]));
 		hook->priority = ntohl(nla_get_be32(ha[NFTA_HOOK_PRIORITY]));
 
-		type = __nft_chain_type_get(family, NFT_CHAIN_T_DEFAULT);
-		if (!type)
-			return -EOPNOTSUPP;
-
 		if (nla[NFTA_CHAIN_TYPE]) {
 			type = nf_tables_chain_type_lookup(net, nla[NFTA_CHAIN_TYPE],
 							   family, true);
@@ -2592,13 +2623,19 @@ static int nft_chain_parse_hook(struct net *net,
 				NL_SET_BAD_ATTR(extack, nla[NFTA_CHAIN_TYPE]);
 				return PTR_ERR(type);
 			}
+		} else {
+			type = nft_chain_type_get(family, NFT_CHAIN_T_DEFAULT);
+			if (!type)
+				return -EOPNOTSUPP;
 		}
+
+		err = -EOPNOTSUPP;
 		if (hook->num >= NFT_MAX_HOOKS || !(type->hook_mask & (1 << hook->num)))
-			return -EOPNOTSUPP;
+			goto out_put_type;
 
 		if (type->type == NFT_CHAIN_T_NAT &&
 		    hook->priority <= NF_IP_PRI_CONNTRACK)
-			return -EOPNOTSUPP;
+			goto out_put_type;
 	} else {
 		if (ha[NFTA_HOOK_HOOKNUM]) {
 			hook->num = ntohl(nla_get_be32(ha[NFTA_HOOK_HOOKNUM]));
@@ -2620,30 +2657,27 @@ static int nft_chain_parse_hook(struct net *net,
 			}
 		} else {
 			type = basechain->type;
+			__module_get(type->owner);
 		}
 	}
 
-	if (!try_module_get(type->owner)) {
-		if (nla[NFTA_CHAIN_TYPE])
-			NL_SET_BAD_ATTR(extack, nla[NFTA_CHAIN_TYPE]);
-		return -ENOENT;
-	}
-
 	hook->type = type;
 
 	INIT_LIST_HEAD(&hook->list);
 	if (nft_base_chain_netdev(family, hook->num)) {
 		err = nft_chain_parse_netdev(net, ha, &hook->list, extack, flags);
-		if (err < 0) {
-			module_put(type->owner);
-			return err;
-		}
+		if (err < 0)
+			goto out_put_type;
 	} else if (ha[NFTA_HOOK_DEV] || ha[NFTA_HOOK_DEVS]) {
-		module_put(type->owner);
-		return -EOPNOTSUPP;
+		err = -EOPNOTSUPP;
+		goto out_put_type;
 	}
 
 	return 0;
+
+out_put_type:
+	module_put(type->owner);
+	return err;
 }
 
 static void nft_chain_release_hook(struct nft_chain_hook *hook)
diff --git a/net/netfilter/nft_chain_nat.c b/net/netfilter/nft_chain_nat.c
index 40e230d8b712..1133f79d8d25 100644
--- a/net/netfilter/nft_chain_nat.c
+++ b/net/netfilter/nft_chain_nat.c
@@ -131,6 +131,7 @@ static void __exit nft_chain_nat_exit(void)
 #ifdef CONFIG_NF_TABLES_INET
 	nft_unregister_chain_type(&nft_chain_nat_inet);
 #endif
+	synchronize_rcu();
 }
 
 module_init(nft_chain_nat_init);
-- 
2.47.3

[PATCH net-next 08/15] netfilter: nft_exthdr: remove redundant op parsing in tcp_set_init

[Pablo Neira Ayuso]

From: Fernando Fernandez Mancera <fmancera@suse.de>

The nft_exthdr_tcp_set_init() function is exclusively used for TCP
options. Therefore, parsing the NFTA_EXTHDR_OP attribute from user
space is redundant, as the operation type must always be
NFT_EXTHDR_OP_TCPOPT.

Remove the unnecessary netlink attribute parsing and hardcode priv->op
to NFT_EXTHDR_OP_TCPOPT, which simplifies the code and reduces the
object size.

./scripts/bloat-o-meter exthdr_before.o exthdr_after.o
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-54 (-54)
Function                                     old     new   delta
nft_exthdr_tcp_set_init                      412     358     -54
Total: Before=6581, After=6527, chg -0.82%

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_exthdr.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index 8abf8e8145ce..3492426dccf7 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -551,7 +551,7 @@ static int nft_exthdr_tcp_set_init(const struct nft_ctx *ctx,
 				   const struct nlattr * const tb[])
 {
 	struct nft_exthdr *priv = nft_expr_priv(expr);
-	u32 offset, len, flags = 0, op = NFT_EXTHDR_OP_IPV6;
+	u32 offset, len, flags = 0;
 	int err;
 
 	if (!tb[NFTA_EXTHDR_SREG] ||
@@ -581,15 +581,11 @@ static int nft_exthdr_tcp_set_init(const struct nft_ctx *ctx,
 		return -EOPNOTSUPP;
 	}
 
-	err = nft_parse_u32_check(tb[NFTA_EXTHDR_OP], U8_MAX, &op);
-	if (err < 0)
-		return err;
-
 	priv->type   = nla_get_u8(tb[NFTA_EXTHDR_TYPE]);
 	priv->offset = offset;
 	priv->len    = len;
 	priv->flags  = flags;
-	priv->op     = op;
+	priv->op     = NFT_EXTHDR_OP_TCPOPT;
 
 	return nft_parse_register_load(ctx, tb[NFTA_EXTHDR_SREG], &priv->sreg,
 				       priv->len);
-- 
2.47.3

[PATCH net-next 09/15] netfilter: nft_exthdr: avoid 60 bytes stack buffer in TCP option mangling

[Pablo Neira Ayuso]

From: Fernando Fernandez Mancera <fmancera@suse.de>

nft_exthdr_tcp_set_eval() and nft_exthdr_tcp_strip_eval() fetched the
TCP header and options through nft_tcp_header_pointer(), which calls
skb_header_pointer() a second time for the full tcphdr_len into a 60
bytes stack buffer. Since commit 28427f368f0e ("netfilter: nft_exthdr:
Fix non-linear header modification") both functions then call
skb_ensure_writable() and re-read the header from skb->data, so the
buffer was never used.

Only the 20 bytes fixed header is needed to learn tcphdr_len, so fetch
just that, validate the length as before, and let skb_ensure_writable()
pull the rest. This also drops the 60 bytes stack buffer from both
functions.

Comparison of performance in mpps:

  op       linear/non-linear     before    after    delta
==================================================================
   set,         non-linear       4.193     4.348    +3.7% mpps
   strip,       non-linear       4.184     4.434    +6.0% mpps
   set,         linear           5.302     5.427    +2.4% mpps
   strip,       linear           5.452     5.527    +1.4% mpss

Comparison of performance in ns/expression:

  op       linear/non-linear     before    after    delta
==================================================================
   set,         non-linear       34.6      25.3     -9.4  ns
   strip,       non-linear       35.2      20.8     -14.3 ns
   set,         linear           23.6      19.7     -3.9  ns
   strip,       linear           22.9      19.8     -3.1  ns

./scripts/bloat-o-meter exthdr_before.o exthdr_after.o
add/remove: 0/1 grow/shrink: 3/0 up/down: 233/-207 (26)
Function                                     old     new   delta
nft_exthdr_tcp_eval                          792     946    +154
nft_exthdr_tcp_strip_eval                    825     891     +66
nft_exthdr_tcp_set_eval                      602     615     +13
nft_tcp_header_pointer.part.constprop        207       -    -207
Total: Before=6527, After=6553, chg +0.40%

This also allow the compiler to inline nft_tcp_header_pointer inside
nft_exthdr_tcp_eval() which could yield some minimal performance gain.

This patch also updates the logic to drop malformed packets, before this
patch NFT_BREAK was used as a verdict instead.

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_exthdr.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index 3492426dccf7..861bab93f435 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -233,16 +233,23 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
 				    struct nft_regs *regs,
 				    const struct nft_pktinfo *pkt)
 {
-	u8 buff[sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE];
 	struct nft_exthdr *priv = nft_expr_priv(expr);
 	unsigned int i, optl, tcphdr_len, offset;
-	struct tcphdr *tcph;
+	struct tcphdr *tcph, _tcph;
 	u8 *opt;
 
-	tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
+	if (pkt->tprot != IPPROTO_TCP || pkt->fragoff)
+		goto err;
+
+	tcph = skb_header_pointer(pkt->skb, nft_thoff(pkt), sizeof(_tcph), &_tcph);
 	if (!tcph)
 		goto err;
 
+	tcphdr_len = __tcp_hdrlen(tcph);
+	if (tcphdr_len < sizeof(*tcph) ||
+	    tcphdr_len > sizeof(*tcph) + MAX_TCP_OPTION_SPACE)
+		goto err;
+
 	if (skb_ensure_writable(pkt->skb, nft_thoff(pkt) + tcphdr_len))
 		goto err;
 
@@ -313,16 +320,23 @@ static void nft_exthdr_tcp_strip_eval(const struct nft_expr *expr,
 				      struct nft_regs *regs,
 				      const struct nft_pktinfo *pkt)
 {
-	u8 buff[sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE];
 	struct nft_exthdr *priv = nft_expr_priv(expr);
 	unsigned int i, tcphdr_len, optl;
-	struct tcphdr *tcph;
+	struct tcphdr *tcph, _tcph;
 	u8 *opt;
 
-	tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
+	if (pkt->tprot != IPPROTO_TCP || pkt->fragoff)
+		goto err;
+
+	tcph = skb_header_pointer(pkt->skb, nft_thoff(pkt), sizeof(_tcph), &_tcph);
 	if (!tcph)
 		goto err;
 
+	tcphdr_len = __tcp_hdrlen(tcph);
+	if (tcphdr_len < sizeof(*tcph) ||
+	    tcphdr_len > sizeof(*tcph) + MAX_TCP_OPTION_SPACE)
+		goto err;
+
 	if (skb_ensure_writable(pkt->skb, nft_thoff(pkt) + tcphdr_len))
 		goto drop;
 
-- 
2.47.3

[PATCH net-next 10/15] selftests: netfilter: fix return value in zone subtest

[Pablo Neira Ayuso]

From: Florian Westphal <fw@strlen.de>

zone subtest prints FAIL, but did not set 'ret=1', so kselftest
framework reports success.

While at it, also fail when "nft -f" returns an error.

Noticed thanks to a (broken) local patch that should have caused this
selftest (resp. the kselftest framework) to report a failure.

Fixes: 33b8aad21ac1 ("selftests: netfilter: add a vrf+conntrack testcase")
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 .../selftests/net/netfilter/conntrack_vrf.sh  | 25 ++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/netfilter/conntrack_vrf.sh b/tools/testing/selftests/net/netfilter/conntrack_vrf.sh
index 207b79932d91..2dbfabaadf4b 100755
--- a/tools/testing/selftests/net/netfilter/conntrack_vrf.sh
+++ b/tools/testing/selftests/net/netfilter/conntrack_vrf.sh
@@ -84,7 +84,7 @@ busywait $BUSYWAIT_TIMEOUT listener_ready "$ns1"
 # as decided by the first iteration of the ruleset.
 test_ct_zone_in()
 {
-ip netns exec "$ns0" nft -f - <<EOF
+if ! ip netns exec "$ns0" nft -f - <<EOF
 table testct {
 	chain rawpre {
 		type filter hook prerouting priority raw;
@@ -105,6 +105,12 @@ table testct {
 	}
 }
 EOF
+	then
+		echo "FAIL: cannot load testct nft ruleset"
+		ret=1
+		return
+	fi
+
 	ip netns exec "$ns1" ping -W 1 -c 1 -I veth0 "$IP0" > /dev/null
 
 	# should be in zone 1, not zone 2
@@ -113,6 +119,7 @@ EOF
 		echo "PASS: entry found in conntrack zone 1"
 	else
 		echo "FAIL: entry not found in conntrack zone 1"
+		ret=1
 		count=$(ip netns exec "$ns0" conntrack -L -s $IP1 -d $IP0 -p icmp --zone 2 2> /dev/null | wc -l)
 		if [ "$count" -eq 1 ]; then
 			echo "FAIL: entry found in zone 2 instead"
@@ -137,7 +144,7 @@ test_masquerade_vrf()
 
 	ip netns exec "$ns0" conntrack -F 2>/dev/null
 
-ip netns exec "$ns0" nft -f - <<EOF
+if ! ip netns exec "$ns0" nft -f - <<EOF
 flush ruleset
 table ip nat {
 	chain rawout {
@@ -158,6 +165,12 @@ table ip nat {
 	}
 }
 EOF
+	then
+		echo "FAIL: cannot load ip nat nft ruleset"
+		ret=1
+		return
+	fi
+
 	if ! ip netns exec "$ns0" ip vrf exec tvrf socat -u -4 STDIN TCP:"$IP1":55555 < /dev/null > /dev/null;then
 		echo "FAIL: connect failure with masquerade + sport rewrite on vrf device"
 		ret=1
@@ -184,7 +197,7 @@ EOF
 test_masquerade_veth()
 {
 	ip netns exec "$ns0" conntrack -F 2>/dev/null
-ip netns exec "$ns0" nft -f - <<EOF
+if ! ip netns exec "$ns0" nft -f - <<EOF
 flush ruleset
 table ip nat {
 	chain postrouting {
@@ -193,6 +206,12 @@ table ip nat {
 	}
 }
 EOF
+	then
+		echo "FAIL: cannot load ip nat postrouting ruleset"
+		ret=1
+		return
+	fi
+
 	if ! ip netns exec "$ns0" ip vrf exec tvrf socat -u -4 STDIN TCP:"$IP1":55555 < /dev/null > /dev/null;then
 		echo "FAIL: connect failure with masquerade + sport rewrite on veth device"
 		ret=1
-- 
2.47.3

[PATCH net-next 11/15] netfilter: nft_compat: restrict raw table targets/matches

[Pablo Neira Ayuso]

From: Florian Westphal <fw@strlen.de>

xt targets and matches declaring .table = "raw" (CT, NOTRACK) rely on
only ever running in the legacy raw table's prerouting/output chains,
ahead of conntrack. nft_compat_chain_validate_dependency() already
enforces this kind of correspondence for "nat" by requiring the chain
type to actually be NAT, but did nothing for "raw": a rule wrapping
the CT target could be loaded into any hook and any priority via
nft_compat, including after the real conntrack hook has already run.

The CT target attaches a conntrack template to the skb when none is
set yet.

Require that a "raw" table dependency actually matches what xtables
enforces via the implict/builtin table dependency.

Fixes: 0ca743a55991 ("netfilter: nf_tables: add compatibility layer for x_tables")
Assisted-by: LLM
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_compat.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 63864b928259..88ddaf47f587 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -14,6 +14,7 @@
 #include <linux/netfilter/nf_tables.h>
 #include <linux/netfilter/nf_tables_compat.h>
 #include <linux/netfilter/x_tables.h>
+#include <linux/netfilter_ipv4.h>
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #include <linux/netfilter_bridge/ebtables.h>
@@ -45,6 +46,20 @@ static int nft_compat_chain_validate_dependency(const struct nft_ctx *ctx,
 			type = NFT_CHAIN_T_NAT;
 		if (basechain->type->type != type)
 			return -EINVAL;
+	} else if (strcmp(tablename, "raw") == 0) {
+		/* iptables-nft emulates the legacy raw table, which only
+		 * ever has prerouting/output chains at this exact priority.
+		 */
+		if (ctx->family != NFPROTO_IPV4 &&
+		    ctx->family != NFPROTO_IPV6)
+			return -EINVAL;
+
+		if (basechain->ops.hooknum != NF_INET_PRE_ROUTING &&
+		    basechain->ops.hooknum != NF_INET_LOCAL_OUT)
+			return -EINVAL;
+
+		if (basechain->ops.priority != NF_IP_PRI_RAW)
+			return -EINVAL;
 	}
 
 	return 0;
-- 
2.47.3

[PATCH net-next 12/15] netfilter: flowtable: set inner protocol when popping tunnel header

[Pablo Neira Ayuso]

From: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

Pop the outer tunnel header and set skb->protocol to the inner protocol
derived from ctx->tun.inner_proto in nf_flow_ip_tunnel_pop(), instead of
bailing out unless the tunnel carries IP-in-IP or IPv6-in-IPv6.
This is a preliminary patch to support IPv4 over IPv6 and SIT tunnel
flowtable offload.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_flow_table_ip.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index c8c29a9a1684..29d1a9f4a362 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -372,6 +372,11 @@ static void nf_flow_ip_tunnel_pop(struct nf_flowtable_ctx *ctx,
 
 	skb_pull(skb, ctx->tun.hdr_size);
 	skb_reset_network_header(skb);
+
+	if (ctx->tun.inner_proto == IPPROTO_IPIP)
+		skb->protocol = htons(ETH_P_IP);
+	else
+		skb->protocol = htons(ETH_P_IPV6);
 }
 
 static bool nf_flow_skb_encap_protocol(struct nf_flowtable_ctx *ctx,
-- 
2.47.3

[PATCH net-next 13/15] netfilter: flowtable: populate tunnel tuple regardless of inner protocol

[Pablo Neira Ayuso]

From: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

Store the outer tunnel addresses and inner protocol in the flow tuple
unconditionally in nf_flow_tuple_encap(), instead of only when the
tunnel is IP-in-IP or IPv6-in-IPv6. Bail out early when the inner
protocol is neither IPPROTO_IPIP nor IPPROTO_IPV6.
This makes the tuple usable for cross-family tunnels such as
IPv4-over-IPv6 and SIT, where the inner protocol differs from the
outer address family.
This is a preliminary patch to support IPv4 over IPv6 and SIT tunnel
flowtable offload.
Please note IPv4 over IPv6 and SIT tunnel flowtable offloading is not
enabled yet.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_flow_table_ip.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 29d1a9f4a362..8379b45b2dad 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -189,22 +189,22 @@ static void nf_flow_tuple_encap(struct nf_flowtable_ctx *ctx,
 		break;
 	}
 
+	if (ctx->tun.inner_proto != IPPROTO_IPIP &&
+	    ctx->tun.inner_proto != IPPROTO_IPV6)
+		return;
+
 	switch (ctx->ether_type) {
 	case htons(ETH_P_IP):
 		iph = (struct iphdr *)(skb_network_header(skb) + offset);
-		if (ctx->tun.inner_proto == IPPROTO_IPIP) {
-			tuple->tun.dst_v4.s_addr = iph->daddr;
-			tuple->tun.src_v4.s_addr = iph->saddr;
-			tuple->tun.inner_proto = IPPROTO_IPIP;
-		}
+		tuple->tun.dst_v4.s_addr = iph->daddr;
+		tuple->tun.src_v4.s_addr = iph->saddr;
+		tuple->tun.inner_proto = ctx->tun.inner_proto;
 		break;
 	case htons(ETH_P_IPV6):
 		ip6h = (struct ipv6hdr *)(skb_network_header(skb) + offset);
-		if (ctx->tun.inner_proto == IPPROTO_IPV6) {
-			tuple->tun.dst_v6 = ip6h->daddr;
-			tuple->tun.src_v6 = ip6h->saddr;
-			tuple->tun.inner_proto = IPPROTO_IPV6;
-		}
+		tuple->tun.dst_v6 = ip6h->daddr;
+		tuple->tun.src_v6 = ip6h->saddr;
+		tuple->tun.inner_proto = ctx->tun.inner_proto;
 		break;
 	default:
 		break;
-- 
2.47.3

[PATCH net-next 14/15] netfilter: flowtable: add encap_proto to flow_offload_tunnel

[Pablo Neira Ayuso]

From: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

Add encap_proto (AF_INET or AF_INET6) to struct flow_offload_tunnel
to allow its use as part of the hash table key during flowtable entry
lookup.
This is a preliminary change to support IPv4 over IPv6 tunneling via
the flowtable infrastructure for software acceleration.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netdevice.h             | 1 +
 include/net/netfilter/nf_flow_table.h | 1 +
 net/ipv4/ipip.c                       | 1 +
 net/ipv6/ip6_tunnel.c                 | 1 +
 net/netfilter/nf_flow_table_ip.c      | 2 ++
 net/netfilter/nf_flow_table_path.c    | 2 ++
 6 files changed, 8 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f61c97231309..fee3728ad666 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -911,6 +911,7 @@ struct net_device_path {
 			};
 
 			u8	inner_proto;
+			u8	encap_proto;
 		} tun;
 		struct {
 			enum {
diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index 5b611efaa3cd..5b22d31cb140 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -118,6 +118,7 @@ struct flow_offload_tunnel {
 	};
 
 	u8	inner_proto;
+	u8	encap_proto;
 };
 
 struct flow_offload_tuple {
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 2ffe64e736e0..15189e1fa068 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -379,6 +379,7 @@ static int ipip_fill_forward_path(struct net_device_path_ctx *ctx,
 	path->tun.src_v4.s_addr = tiph->saddr;
 	path->tun.dst_v4.s_addr = tiph->daddr;
 	path->tun.inner_proto = IPPROTO_IPIP;
+	path->tun.encap_proto = AF_INET;
 	path->tun.dst = &rt->dst;
 	path->dev = ctx->dev;
 
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index d5ff50a2ac01..64a184bca467 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1868,6 +1868,7 @@ static int ip6_tnl_fill_forward_path(struct net_device_path_ctx *ctx,
 		path->tun.src_v6 = fl6.saddr;
 		path->tun.dst_v6 = fl6.daddr;
 		path->tun.inner_proto = IPPROTO_IPV6;
+		path->tun.encap_proto = AF_INET6;
 		path->tun.dst = dst;
 		path->dev = ctx->dev;
 		ctx->dev = dst->dev;
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 8379b45b2dad..29a769736dbf 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -199,12 +199,14 @@ static void nf_flow_tuple_encap(struct nf_flowtable_ctx *ctx,
 		tuple->tun.dst_v4.s_addr = iph->daddr;
 		tuple->tun.src_v4.s_addr = iph->saddr;
 		tuple->tun.inner_proto = ctx->tun.inner_proto;
+		tuple->tun.encap_proto = AF_INET;
 		break;
 	case htons(ETH_P_IPV6):
 		ip6h = (struct ipv6hdr *)(skb_network_header(skb) + offset);
 		tuple->tun.dst_v6 = ip6h->daddr;
 		tuple->tun.src_v6 = ip6h->saddr;
 		tuple->tun.inner_proto = ctx->tun.inner_proto;
+		tuple->tun.encap_proto = AF_INET6;
 		break;
 	default:
 		break;
diff --git a/net/netfilter/nf_flow_table_path.c b/net/netfilter/nf_flow_table_path.c
index d90013685bf1..841f099434d9 100644
--- a/net/netfilter/nf_flow_table_path.c
+++ b/net/netfilter/nf_flow_table_path.c
@@ -138,6 +138,7 @@ static int nft_dev_path_info(struct net_device_path_stack *stack,
 				info->tun.dst_v6 = path->tun.dst_v6;
 				info->tun.inner_proto = path->tun.inner_proto;
 				info->tun_dst = path->tun.dst;
+				info->tun.encap_proto = path->tun.encap_proto;
 				info->num_tuns++;
 			} else {
 				if (info->num_encaps >= NF_FLOW_TABLE_ENCAP_MAX)
@@ -249,6 +250,7 @@ static int nft_dev_forward_path(const struct nft_pktinfo *pkt,
 		route->tuple[!dir].in.tun.src_v6 = info.tun.dst_v6;
 		route->tuple[!dir].in.tun.dst_v6 = info.tun.src_v6;
 		route->tuple[!dir].in.tun.inner_proto = info.tun.inner_proto;
+		route->tuple[!dir].in.tun.encap_proto = info.tun.encap_proto;
 		route->tuple[!dir].in.num_tuns = info.num_tuns;
 		dst_release(route->tuple[dir].dst);
 		route->tuple[dir].dst = info.tun_dst;
-- 
2.47.3

[PATCH net-next 15/15] netfilter: flowtable: unify tunnel push for IPv4 and IPv6

[Pablo Neira Ayuso]

From: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

Refactor nf_flow_tunnel_ipip_push() and nf_flow_tunnel_ip6ip6_push()
into nf_flow_tunnel_ip_push() and nf_flow_tunnel_ip6_push(), keying the
inner header handling off tuple->tun.inner_proto so both IP-in-IP and
IPv6-in-IPv6 inner protocols are supported regardless of the outer
address family. Replace nf_flow_tunnel_v4_push() and
nf_flow_tunnel_v6_push() with a single nf_flow_tunnel_push() that
dispatches on tuple->tun.encap_proto, and set skb->protocol explicitly
after pushing the outer header.
This is a preliminary patch to support IPv4 over IPv6 and SIT tunnel
flowtable offload.
Please note IPv4 over IPv6 and SIT tunnel flowtable offloading is not
enabled yet.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_flow_table_ip.c | 143 +++++++++++++++++++------------
 1 file changed, 87 insertions(+), 56 deletions(-)

diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 29a769736dbf..e13f9397e8cb 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -587,23 +587,44 @@ static int nf_flow_pppoe_push(struct sk_buff *skb, u16 id,
 	return 0;
 }
 
-static int nf_flow_tunnel_ipip_push(struct net *net, struct sk_buff *skb,
-				    struct flow_offload_tuple *tuple,
-				    struct dst_entry *dst, __be32 *ip_daddr)
+static int nf_flow_tunnel_ip_push(struct net *net, struct sk_buff *skb,
+				  struct flow_offload_tuple *tuple,
+				  struct dst_entry *dst,
+				  union nf_inet_addr **ip_addr)
 {
-	struct iphdr *iph = (struct iphdr *)skb_network_header(skb);
-	struct rtable *rt = dst_rtable(dst);
-	u8 tos = iph->tos, ttl = iph->ttl;
-	__be16 frag_off = iph->frag_off;
-	u32 headroom = sizeof(*iph);
+	__be16 frag_off = 0;
+	struct iphdr *iph;
+	u8 tos = 0, ttl;
+	u32 headroom;
 	int err;
 
+	switch (tuple->tun.inner_proto) {
+	case IPPROTO_IPV6: {
+		struct ipv6hdr *ip6h;
+
+		ip6h = (struct ipv6hdr *)skb_network_header(skb);
+		tos = ipv6_get_dsfield(ip6h);
+		ttl = ip6h->hop_limit;
+		frag_off = htons(IP_DF);
+		break;
+	}
+	case IPPROTO_IPIP:
+		iph = (struct iphdr *)skb_network_header(skb);
+		frag_off = iph->frag_off;
+		tos = iph->tos;
+		ttl = iph->ttl;
+		break;
+	default:
+		return 0;
+	}
+
 	err = iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4);
 	if (err)
 		return err;
 
-	skb_set_inner_ipproto(skb, IPPROTO_IPIP);
-	headroom += LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len;
+	skb_set_inner_ipproto(skb, tuple->tun.inner_proto);
+	headroom = sizeof(*iph) + LL_RESERVED_SPACE(dst->dev) +
+		   dst->header_len;
 	err = skb_cow_head(skb, headroom);
 	if (err)
 		return err;
@@ -614,11 +635,12 @@ static int nf_flow_tunnel_ipip_push(struct net *net, struct sk_buff *skb,
 	/* Push down and install the IP header. */
 	skb_push(skb, sizeof(*iph));
 	skb_reset_network_header(skb);
+	skb->protocol = htons(ETH_P_IP);
 
 	iph = ip_hdr(skb);
 	iph->version	= 4;
 	iph->ihl	= sizeof(*iph) >> 2;
-	iph->frag_off	= ip_mtu_locked(&rt->dst) ? 0 : frag_off;
+	iph->frag_off	= ip_mtu_locked(dst) ? 0 : frag_off;
 	iph->protocol	= tuple->tun.inner_proto;
 	iph->tos	= tos;
 	iph->daddr	= tuple->tun.src_v4.s_addr;
@@ -628,81 +650,91 @@ static int nf_flow_tunnel_ipip_push(struct net *net, struct sk_buff *skb,
 	__ip_select_ident(net, iph, skb_shinfo(skb)->gso_segs ?: 1);
 	ip_send_check(iph);
 
-	*ip_daddr = tuple->tun.src_v4.s_addr;
-
-	return 0;
-}
-
-static int nf_flow_tunnel_v4_push(struct net *net, struct sk_buff *skb,
-				  struct flow_offload_tuple *tuple,
-				  struct dst_entry *dst,  __be32 *ip_daddr)
-{
-	if (tuple->tun_num)
-		return nf_flow_tunnel_ipip_push(net, skb, tuple, dst, ip_daddr);
+	*ip_addr = (union nf_inet_addr *)&tuple->tun.src_v4;
 
 	return 0;
 }
 
-static int nf_flow_tunnel_ip6ip6_push(struct net *net, struct sk_buff *skb,
-				      struct flow_offload_tuple *tuple,
-				      struct dst_entry *dst,
-				      struct in6_addr **ip6_daddr)
+static int nf_flow_tunnel_ip6_push(struct net *net, struct sk_buff *skb,
+				   struct flow_offload_tuple *tuple,
+				   struct dst_entry *dst,
+				   union nf_inet_addr **ip_addr)
 {
-	struct ipv6hdr *ip6h = (struct ipv6hdr *)skb_network_header(skb);
-	__u8 dsfield = ipv6_get_dsfield(ip6h);
-	struct rtable *rt = dst_rtable(dst);
 	struct flowi6 fl6 = {
 		.daddr = tuple->tun.src_v6,
 		.saddr = tuple->tun.dst_v6,
-		.flowi6_proto = IPPROTO_IPV6,
+		.flowi6_proto = tuple->tun.inner_proto,
 	};
-	u8 hop_limit = ip6h->hop_limit;
+	u8 hop_limit, dsfield;
+	struct ipv6hdr *ip6h;
 	int err, mtu;
 	u32 headroom;
 
+	switch (tuple->tun.inner_proto) {
+	case IPPROTO_IPIP: {
+		struct iphdr *iph = (struct iphdr *)skb_network_header(skb);
+
+		dsfield = ipv4_get_dsfield(iph);
+		hop_limit = iph->ttl;
+		break;
+	}
+	case IPPROTO_IPV6:
+		ip6h = (struct ipv6hdr *)skb_network_header(skb);
+		dsfield = ipv6_get_dsfield(ip6h);
+		hop_limit = ip6h->hop_limit;
+		break;
+	default:
+		return 0;
+	}
+
 	err = iptunnel_handle_offloads(skb, SKB_GSO_IPXIP6);
 	if (err)
 		return err;
 
-	skb_set_inner_ipproto(skb, IPPROTO_IPV6);
-	headroom = sizeof(*ip6h) + LL_RESERVED_SPACE(rt->dst.dev) +
-		   rt->dst.header_len;
+	skb_set_inner_ipproto(skb, tuple->tun.inner_proto);
+	headroom = sizeof(*ip6h) + LL_RESERVED_SPACE(dst->dev) +
+		   dst->header_len;
 	err = skb_cow_head(skb, headroom);
 	if (err)
 		return err;
 
 	skb_scrub_packet(skb, true);
-	mtu = dst_mtu(&rt->dst) - sizeof(*ip6h);
+	mtu = dst_mtu(dst) - sizeof(*ip6h);
 	mtu = max(mtu, IPV6_MIN_MTU);
 	skb_dst_update_pmtu_no_confirm(skb, mtu);
 
 	skb_push(skb, sizeof(*ip6h));
 	skb_reset_network_header(skb);
+	skb->protocol = htons(ETH_P_IPV6);
 
 	ip6h = ipv6_hdr(skb);
 	ip6_flow_hdr(ip6h, dsfield,
 		     ip6_make_flowlabel(net, skb, fl6.flowlabel, true, &fl6));
 	ip6h->hop_limit = hop_limit;
-	ip6h->nexthdr = IPPROTO_IPV6;
+	ip6h->nexthdr = tuple->tun.inner_proto;
 	ip6h->daddr = tuple->tun.src_v6;
 	ip6h->saddr = tuple->tun.dst_v6;
 	ipv6_hdr(skb)->payload_len = htons(skb->len - sizeof(*ip6h));
 	IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);
 
-	*ip6_daddr = &tuple->tun.src_v6;
+	*ip_addr = (union nf_inet_addr *)&tuple->tun.src_v6;
 
 	return 0;
 }
 
-static int nf_flow_tunnel_v6_push(struct net *net, struct sk_buff *skb,
-				  struct flow_offload_tuple *tuple,
-				  struct dst_entry *dst,
-				  struct in6_addr **ip6_daddr)
+static int nf_flow_tunnel_push(struct net *net, struct sk_buff *skb,
+			       struct flow_offload_tuple *tuple,
+			       struct dst_entry *dst,
+			       union nf_inet_addr **ip_addr)
 {
-	if (tuple->tun_num)
-		return nf_flow_tunnel_ip6ip6_push(net, skb, tuple, dst, ip6_daddr);
-
-	return 0;
+	switch (tuple->tun.encap_proto) {
+	case AF_INET:
+		return nf_flow_tunnel_ip_push(net, skb, tuple, dst, ip_addr);
+	case AF_INET6:
+		return nf_flow_tunnel_ip6_push(net, skb, tuple, dst, ip_addr);
+	default:
+		return 0;
+	}
 }
 
 static int nf_flow_encap_push(struct sk_buff *skb,
@@ -809,10 +841,10 @@ static int nf_flow_queue_xmit4(struct sk_buff *skb,
 	struct flow_offload_tuple *other_tuple;
 	enum flow_offload_tuple_dir dir;
 	struct nf_flow_xmit xmit = {};
+	union nf_inet_addr *ip_addr;
 	struct flow_offload *flow;
 	struct neighbour *neigh;
 	struct rtable *rt;
-	__be32 ip_daddr;
 
 	if (unlikely(tuplehash->tuple.xmit_type == FLOW_OFFLOAD_XMIT_XFRM)) {
 		rt = dst_rtable(tuplehash->tuple.dst_cache);
@@ -825,10 +857,10 @@ static int nf_flow_queue_xmit4(struct sk_buff *skb,
 	dir = tuplehash->tuple.dir;
 	flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
 	other_tuple = &flow->tuplehash[!dir].tuple;
-	ip_daddr = other_tuple->src_v4.s_addr;
+	ip_addr = (union nf_inet_addr *)&other_tuple->src_v4;
 
-	if (nf_flow_tunnel_v4_push(state->net, skb, other_tuple,
-				   tuplehash->tuple.dst_cache, &ip_daddr) < 0)
+	if (nf_flow_tunnel_push(state->net, skb, other_tuple,
+				tuplehash->tuple.dst_cache, &ip_addr) < 0)
 		return NF_DROP;
 
 	switch (tuplehash->tuple.xmit_type) {
@@ -839,7 +871,7 @@ static int nf_flow_queue_xmit4(struct sk_buff *skb,
 			flow_offload_teardown(flow);
 			return NF_DROP;
 		}
-		neigh = ip_neigh_gw4(rt->dst.dev, rt_nexthop(rt, ip_daddr));
+		neigh = ip_neigh_gw4(rt->dst.dev, rt_nexthop(rt, ip_addr->ip));
 		if (IS_ERR(neigh)) {
 			flow_offload_teardown(flow);
 			return NF_DROP;
@@ -1137,7 +1169,7 @@ static int nf_flow_queue_xmit6(struct sk_buff *skb,
 	struct flow_offload_tuple *other_tuple;
 	enum flow_offload_tuple_dir dir;
 	struct nf_flow_xmit xmit = {};
-	struct in6_addr *ip6_daddr;
+	union nf_inet_addr *ip_addr;
 	struct flow_offload *flow;
 	struct neighbour *neigh;
 	struct rt6_info *rt;
@@ -1153,11 +1185,10 @@ static int nf_flow_queue_xmit6(struct sk_buff *skb,
 	dir = tuplehash->tuple.dir;
 	flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
 	other_tuple = &flow->tuplehash[!dir].tuple;
-	ip6_daddr = &other_tuple->src_v6;
+	ip_addr = (union nf_inet_addr *)&other_tuple->src_v6;
 
-	if (nf_flow_tunnel_v6_push(state->net, skb, other_tuple,
-				   tuplehash->tuple.dst_cache,
-				   &ip6_daddr) < 0)
+	if (nf_flow_tunnel_push(state->net, skb, other_tuple,
+				tuplehash->tuple.dst_cache, &ip_addr) < 0)
 		return NF_DROP;
 
 	switch (tuplehash->tuple.xmit_type) {
@@ -1168,7 +1199,7 @@ static int nf_flow_queue_xmit6(struct sk_buff *skb,
 			flow_offload_teardown(flow);
 			return NF_DROP;
 		}
-		neigh = ip_neigh_gw6(rt->dst.dev, rt6_nexthop(rt, ip6_daddr));
+		neigh = ip_neigh_gw6(rt->dst.dev, rt6_nexthop(rt, &ip_addr->in6));
 		if (IS_ERR(neigh)) {
 			flow_offload_teardown(flow);
 			return NF_DROP;
-- 
2.47.3

Re: [PATCH net-next 01/15] netfilter: disable br_netfilter in user namespaces

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next 01/15] netfilter: disable br_netfilter in user namespaces

[Pablo Neira Ayuso]

Hi,

On Sun, Oct 18, 2026 at 01:03:17AM +0000, netdev-bot+sinfo@kernel.org wrote:
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.

Yes, this contains some low priority fixes as well as enhancements:

1) For the br_netfilter patch, existing limitations are known and
   issues keep popping from time to time, let's prepare for deprecation.

2) Found by Jakub in NIPA.

3) For the IPVS sync message overflow: found via manual code
   inspection. It is theoretical for modern kernels - the description
   notes it "can only happen in very old 2.6 kernels," so it was not
   triggered in any current production or test environment.

4 and 5) unclear if it was found in production, but it comes from
   third party software developer with realistic use-cases.

6) For the conntrack gc worker UaF: found via manual code inspection.
   It is described as "unlikely," so it is theoretical - not
   observed as a triggered crash or stack trace.

7) For the nf_tables chain types UaF on module removal: found via
   manual code inspection. The race window (module removal while RCU
   readers are active) makes it theoretical in practice; no stack trace
   or production report is referenced.

8 and 9) Simplify nft_exthdr tcp option parsing.

10) fixes a conntrack_vrf.sh selftest which has been seen to be
   reporting bogus issues.

11) was found via LLM that I am aware of.

12) is preparation work from Lorenzo Bianconi towards adding
    IPv4-over-IPv6 and vice-versa. 

This answer to this email has been assisted by LLM.

> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH net-next 01/15] netfilter: disable br_netfilter in user namespaces

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] tools/testing/selftests/net/netfilter/br_netfilter_queue.sh
  re-execs itself under `unshare -n` (line 40) and then runs `sysctl…

--

netfilter: disable br_netfilter in user namespaces

This commit disables br_netfilter in network namespaces that are not
owned by init_user_ns. In those namespaces it skips registration of the
bridge-nf sysctls and of the br_netfilter hooks. It also adds a boot-time
warning that the module is slated for removal in 2027, and updates the
bridge documentation to match.

> diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
> index 0a394e5f43916..e15355ff215a7 100644
> --- a/net/bridge/br_netfilter_hooks.c
> +++ b/net/bridge/br_netfilter_hooks.c

[ ... ]

>  static int __net_init brnf_init_net(struct net *net)
>  {
> +	if (net->user_ns != &init_user_ns) {
> +		struct brnf_net *brnet = net_generic(net, brnf_net_id);
> +
> +		brnet->in_userns = true;
> +		pr_warn_once("br_netfilter no longer supported in user namespaces\n");
> +		return 0;
> +	}
> +
>  	return br_netfilter_sysctl_init_net(net);
>  }

[Severity: Medium]
Should tools/testing/selftests/net/netfilter/br_netfilter_queue.sh be
updated along with this change?

That script re-execs itself in a new netns via unshare -n. The new netns
inherits the caller's user namespace:

    [ "${1}" != "run" ] && { unshare -n "${0}" run; exit $?; }

It then treats a missing sysctl as a hard failure:

    modprobe -q br_netfilter

    sysctl net.bridge.bridge-nf-call-iptables=1 || exit 1

Suppose the selftests run inside a non-initial user namespace, for
example a rootless container or a CI job under unshare -r. Then the
netns created by unshare -n has user_ns != &init_user_ns, and
brnf_init_net() returns before registering the sysctls. The sysctl
write fails and the script exits 1, so the test reports FAIL instead of
SKIP.

Before this patch the sysctl was present and writable by userns root,
so the test passed in that environment.

The sibling test br_netfilter.sh already treats the same condition as a
skip:

    if ! ip netns exec "$ns0" sysctl -q net.bridge.bridge-nf-call-iptables=1; then
    	echo "SKIP: bridge netfilter not available"
    	ret=$ksft_skip
    fi

Could br_netfilter_queue.sh use the same handling and exit with
$ksft_skip here?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org

Re: [PATCH net-next 02/15] selftests: netfilter: nft_queue.sh: only queue icmp echo request/reply

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit fixes a reproducible flake in the bridge NFQUEUE
  selftest (about one failure a day in NIPA), but its message has no…

--

selftests: netfilter: nft_queue.sh: only queue icmp echo request/reply

The patch changes the ipv4 nfq rule in load_ruleset() so it only queues
icmp echo request/reply, which the icmpv6 rule already does. ICMP time
exceeded messages left over after the stress test no longer inflate the
bridge queue packet counts.

> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> Reviewed-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

[Severity: Low]
This isn't a bug, but should this have a Fixes: tag?

The flake looks like it started with:

commit 8a2cfe7951f6 ("selftests: nft_queue.sh: add a bridge queue test")

That commit added test_queue_bridge() and called it at the end of the
script, after test_queue_stress():

    test_queue_stress
    ...
    # turns router into a bridge
    test_queue_bridge

It also loaded the existing "ip protocol icmp queue bypass" rule for the
bridge family, and then checked exact counts:

    load_ruleset "bridge" "filter" 10
    test_queue 10 "bridge"

Before that commit, this rule was only loaded for the inet tests. Those
run before the stress test, so the delayed reassembly timeout ICMP errors
were never counted.

8a2cfe7951f6 says it was added ahead of a kernel fix that checks bridge
ports are still valid on reinject. That fix could be picked up for
stable. If the test goes to stable trees without this follow-up, the
bridge test would flake there as well.

Would something like this be worth adding?

Fixes: 8a2cfe7951f6 ("selftests: nft_queue.sh: add a bridge queue test")

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org

Re: [PATCH net-next 04/15] netfilter: nf_queue: limit the hook drop flush to the unregistered hook point

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Use-after-free of module code: after this patch, `rmmod
  br_netfilter` no longer drops queued packets that point into…
- [Medium] Conntrack module unload can now stall on queued packets, and
  the commit message does not mention it.
- [Low] The commit message has no Fixes: tag.

--

netfilter: nf_queue: limit the hook drop flush to the unregistered hook point

This patch passes the nf_hook_ops being unregistered down to the nfqueue
handler. It adds nf_hook_cmp() so that nfqnl_nf_hook_drop() only flushes
queued entries whose state.pf/state.hook match the hook point of the
removed hook. Callers that pass NULL (nf_ct_iterate_destroy() and
__nft_ct_set_destroy()) still flush every entry.

Link: https://lore.kernel.org/netfilter-devel/CABhP=tYtCyr-qzL==98nkmDVZr6x+f8E26V1AAOpMXgZgt0vAQ@mail.gmail.com/

> Before commit 039b40ee5854 ("netfilter: nf_queue: only call
> synchronize_net twice if nf_queue is active") nf_queue_nf_hook_drop()
> received the removed hook and nfqueue only dropped the entries queued by
> that hook.

[ ... ]

> Suggested-by: Florian Westphal <fw@strlen.de>
> Link: https://lore.kernel.org/netfilter-devel/CABhP=tYtCyr-qzL==98nkmDVZr6x+f8E26V1AAOpMXgZgt0vAQ@mail.gmail.com/
> Assisted-by: LLM
> Signed-off-by: Antonio Ojea <aojea@google.com>

[Severity: Low]
Should this have a Fixes: tag?

The commit message describes a user-visible problem: deleting an unrelated
base chain drops queued packets, and the next verdict fails with -ENOENT.
It also names 039b40ee5854 as the commit that removed the per-hook
comparison.

Without the trailer, stable maintainers may miss this fix. The message
also does not say the omission is deliberate.

> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index 477fc632657f4..90d887546f7db 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -1566,9 +1566,39 @@ static struct notifier_block nfqnl_dev_notifier = {

[ ... ]

> +static int nf_hook_cmp(struct nf_queue_entry *entry, unsigned long ops_ptr)
> +{
> +	const struct nf_hook_ops *ops = (const struct nf_hook_ops *)ops_ptr;
> +	unsigned int hooknum = ops->hooknum;
> +	u8 pf = ops->pf;
> +
> +	/* same translation as nf_static_key_inc() */
> +	if (pf == NFPROTO_INET && hooknum == NF_INET_INGRESS) {
> +		pf = NFPROTO_NETDEV;
> +		hooknum = NF_NETDEV_INGRESS;
> +	}
> +
> +	if (entry->state.hook != hooknum)
> +		return 0;
> +
> +	if (pf == NFPROTO_INET)
> +		return entry->state.pf == NFPROTO_IPV4 ||
> +		       entry->state.pf == NFPROTO_IPV6;
> +
> +	return entry->state.pf == pf;
> +}

[Severity: High]
After this change, can packets whose okfn points into br_netfilter.ko stay
queued after that module is unloaded?

br_netfilter calls NF_HOOK() at hook points where it registers no hook of
its own, and passes its own functions as okfn:

net/bridge/br_netfilter_hooks.c:br_nf_forward() {
    ...
	NF_HOOK(pf, NF_INET_FORWARD, state->net, NULL, skb,
		brnf_get_logical_dev(skb, state->in, state->net),
		parent,	br_nf_forward_finish);
    ...
}

br_nf_forward_arp() does the same at NFPROTO_ARP/NF_ARP_FORWARD.
br_nf_post_routing() does it at NF_INET_POST_ROUTING with
br_nf_dev_queue_xmit.

br_nf_ops only has the NFPROTO_BRIDGE hooks and the IPv4/IPv6
NF_INET_PRE_ROUTING hooks. On rmmod br_netfilter the teardown is:

br_netfilter_fini()
  unregister_pernet_subsys()
    brnf_exit_net()
      nf_unregister_net_hooks(net, br_nf_ops, ARRAY_SIZE(br_nf_ops))
        __nf_unregister_net_hook()
          nf_queue_nf_hook_drop(net, reg)
            nfqnl_nf_hook_drop()
              nf_hook_cmp()

Suppose a bridged packet is queued by an ip/ip6 FORWARD or POSTROUTING
NFQUEUE rule, or an arp FORWARD rule. nf_hook_cmp() then returns 0 for
every hook in br_nf_ops.

For example, the NFPROTO_BRIDGE NF_BR_FORWARD hook has the same hooknum as
NF_INET_FORWARD, but entry->state.pf is NFPROTO_IPV4 rather than
NFPROTO_BRIDGE.

Before this patch, each unregistration flushed every entry in the netns.
That flush is what removed these packets, since queue entries take no
module reference.

When userspace later sends NF_ACCEPT, the IPv4 FORWARD hook array has not
changed. hook_index is still valid, so nf_reinject() reaches:

	entry->state.okfn(entry->state.net, entry->state.sk, skb);

okfn is br_nf_forward_finish(), which is in the freed module text.

Is a flush-all, such as nf_queue_nf_hook_drop(net, NULL), needed in the
br_netfilter exit path, or some other flush at module teardown for callers
whose okfn is module code?

> +
> +static void nfqnl_nf_hook_drop(struct net *net, const struct nf_hook_ops *ops)
>  {
>  	struct nfnl_queue_net *q = nfnl_queue_pernet(net);
> +	nfqnl_cmpfn cmpfn = NULL;
>  	int i;
>  
>  	/* This function is also called on net namespace error unwind,
> @@ -1581,12 +1611,19 @@ static void nfqnl_nf_hook_drop(struct net *net)
>  	if (!q)
>  		return;
>  
> +	/* ops is NULL when the caller is not unregistering a hook but
> +	 * removing an object queued packets may reference, every entry
> +	 * has to go
> +	 */
> +	if (ops)
> +		cmpfn = nf_hook_cmp;
> +
>  	for (i = 0; i < INSTANCE_BUCKETS; i++) {
>  		struct nfqnl_instance *inst;
>  		struct hlist_head *head = &q->instance_table[i];
>  
>  		hlist_for_each_entry_rcu(inst, head, hlist)
> -			nfqnl_flush(inst, NULL, 0);
> +			nfqnl_flush(inst, cmpfn, (unsigned long)ops);

[Severity: Medium]
Could rmmod nf_conntrack now block while userspace holds verdicts for
queued packets?

When the last conntrack user goes away, nf_ct_netns_do_put() unregisters
ipv4_conntrack_ops/ipv6_conntrack_ops. Those hooks are only at
PRE_ROUTING, LOCAL_IN, LOCAL_OUT and POST_ROUTING.

Before this patch, that unregistration flushed every queued entry. This
released the nf_conn references held through skb->_nfct.

With the filtered flush, packets queued at NF_INET_FORWARD stay queued, for
example from iptables -A FORWARD -j NFQUEUE or an nft forward queue rule.
Their conntracks keep cnet->count above zero.

nf_conntrack_standalone_fini() only calls nf_conntrack_cleanup_start() and
unregister_pernet_subsys(&nf_conntrack_net_ops). It never calls
nf_queue_nf_hook_drop(net, NULL). nf_conntrack_cleanup_net_list() then
keeps looping:

i_see_dead_people:
	busy = 0;
	list_for_each_entry(net, net_exit_list, exit_list) {
		...
		if (atomic_read(&cnet->count) != 0)
			busy = 1;
	}
	if (busy) {
		DEBUG_NET_WARN_ONCE(time_after(jiffies, start + 60 * HZ),
				    "conntrack cleanup blocked for 60s");
		schedule();
		goto i_see_dead_people;
	}

This runs while pernet_ops_rwsem is held for write. Netns creation and
teardown would stall until a verdict is sent or the queue socket is
closed.

The commit message says nf_ct_iterate_destroy() keeps the NULL flush on
module exit. That path covers helpers and NAT, not nf_conntrack itself,
and the message does not mention this case.

Should nf_conntrack's exit path also flush with NULL?

>  	}
>  }

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org

Re: [PATCH net-next 05/15] selftests: netfilter: nft_queue: check the scope of the hook drop flush

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new test's failure does not reach the script's exit
  status.
- [Low] The 'keep' case cannot tell the two halves of nf_hook_cmp()
  apart.
- [Low] In hook_drop_case() the exit status of the command under test,
  `ip netns exec "$ns1" nft "$@"` (line 932), is never checked, and the…
- [Low] hook_drop_case() ignores the return value of `busywait
  "$BUSYWAIT_TIMEOUT" hook_drop_all_queued "$packets"` (line 930).

--

selftests: netfilter: nft_queue: check the scope of the hook drop flush

This adds test_hook_drop_scope to nft_queue.sh. The test queues 20 UDP
packets to the nf_queue helper, which holds the verdicts, and deletes a
base chain while the packets wait. It then checks whether the packets
were kept or dropped. It also adds a -H option to nf_queue that holds
verdicts until SIGUSR1 is received.

> diff --git a/tools/testing/selftests/net/netfilter/nft_queue.sh b/tools/testing/selftests/net/netfilter/nft_queue.sh
> index c8d1f2eb41339..a07652813a4b0 100755
> --- a/tools/testing/selftests/net/netfilter/nft_queue.sh
> +++ b/tools/testing/selftests/net/netfilter/nft_queue.sh

[ ... ]

> +hook_drop_case()
> +{

[ ... ]

> +	ip netns exec "$ns1" bash -c \
> +		"for i in \$(seq $packets); do echo x > /dev/udp/127.0.0.1/12345; done"
> +
> +	busywait "$BUSYWAIT_TIMEOUT" hook_drop_all_queued "$packets"

[Severity: Low]
What happens if this busywait times out? On timeout, loopy_wait() in
lib.sh just returns 1. It does not abort the caller:

tools/testing/selftests/net/lib.sh:loopy_wait() {
    ...
		if ((current_time - start_time > timeout_ms)); then
			echo -n "$out"
			return 1
		fi
    ...
}

The return value is ignored here. So the test still deletes the chain
and classifies the result when fewer than 20 packets were queued, for
example when some of the sends were lost before they reached the queue.

The drop classification further down only checks for zero:

	elif [ "$queued" = "0" ] && [ "$delivered" = "0" ]; then
		result="drop"

Could the drop case then report PASS with only part of the batch queued,
or with nothing queued at all? In the keep case the same situation
would be reported as inconsistent.

> +
> +	ip netns exec "$ns1" nft "$@"
> +	queued=$(hook_drop_queued)

[Severity: Low]
Should the exit status of this nft command be checked?

Suppose the deletion fails, or test_hook_drop_scope() passes the wrong
arguments. Then no hook is unregistered and the 20 packets stay queued.
SIGUSR1 releases them, the input counter reaches 20 and nf_queue prints
"20 packets total". The keep case would report PASS even though no
chain was deleted.

Other deletions in this file do check the result, for example in
test_udp_gro_ct():

	if ! ip netns exec "$nsrouter" nft delete table inet udpq; then
		echo "FAIL: Could not delete udpq table"
		ret=1

[ ... ]

> +test_hook_drop_scope()
> +{
> +	# the hook array of another hook point changes, queued packets stay
> +	hook_drop_case keep "base chain removed at another hook point" \
> +		delete chain ip6 unrelated prerouting

[Severity: Low]
Does this keep case test the hook number check and the family check in
nf_hook_cmp() separately? The queued packets are IPv4 (sent to
127.0.0.1) at NF_INET_LOCAL_OUT. The deleted chain differs from them in
both family (ip6) and hook (prerouting):

net/netfilter/nfnetlink_queue.c:nf_hook_cmp() {
    ...
	if (entry->state.hook != hooknum)
		return 0;

	if (pf == NFPROTO_INET)
		return entry->state.pf == NFPROTO_IPV4 ||
		       entry->state.pf == NFPROTO_IPV6;

	return entry->state.pf == pf;
}

If nf_hook_cmp() compared only the hook number, or only the family, it
would still keep the entries and this case would pass. The drop case
matches on both fields, so it doesn't isolate either check.

Would it make sense to add a chain at the same hook in another family
(for example ip6 output)? A chain at another hook in the same family
(for example ip prerouting) would cover the other check.

The testing notes of the previous patch, "netfilter: nf_queue: limit the
hook drop flush to the unregistered hook point", say that deleting an
ip6 table with an output base chain does not drop the packets. That
case isn't encoded in this test.

[ ... ]

> @@ -909,6 +1016,7 @@ test_queue_stress
>  # should be last, adds vrf device in ns1 and changes routes
>  test_icmp_vrf
>  test_queue_removal
> +test_hook_drop_scope
>  
>  # turns router into a bridge
>  test_queue_bridge

[Severity: Medium]
Does a failure in test_hook_drop_scope() reach the exit status of the
script? hook_drop_case() records a failure only by setting the global
ret=1.

test_queue_bridge runs afterwards and calls test_queue() twice.
test_queue() does this:

tools/testing/selftests/net/netfilter/nft_queue.sh:test_queue() {
    ...
	wait
	ret=$?
    ...
}

A bare wait returns 0, so ret is reset to 0 whenever the bridge tests
pass. The script then reports success with exit $ret.

The FAIL line still goes to stderr, but kselftest runners decide pass
or fail from the exit code. The same reset already affects the earlier
tests in this sequence.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org

Re: [PATCH net-next 07/15] netfilter: nf_tables: switch chain_types array to rcu

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Commit "netfilter: nf_tables: switch chain_types array to rcu"
  fixes a module-memory use-after-free but has no Fixes: tag and no Cc:…

--

netfilter: nf_tables: switch chain_types array to rcu

This converts the global chain_type array in nf_tables_api.c to RCU and
takes the module reference on the chain type owner inside the RCU read
section. That closes a window where a transaction could dereference an
nft_chain_type freed by a concurrent rmmod of nft_chain_nat, and
nft_chain_nat_exit() now calls synchronize_rcu() after unregistering.

> Reported-by: Vega <vega@nebusec.ai>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

[Severity: Low]
Should this patch carry a Fixes: tag?

Before this patch, two lookups happened before try_module_get(type->owner),
with only the pernet commit_mutex held:

  - __nf_tables_chain_type_lookup() dereferenced t->name
  - nft_chain_parse_hook() read type->hook_mask and type->type

At the same time, nft_unregister_chain_type() cleared the slot under
nfnl_lock(NFNL_SUBSYS_NFTABLES) only.

The window seems to have been opened by commit f102d66b335a ("netfilter:
nf_tables: use dedicated mutex to guard transactions"). That commit moved
nfnl_unlock() ahead of the batch message loop in nfnetlink_rcv_batch().
It kept the nfnl mutex for chain type (un)registration.

Would this trailer be appropriate?

Fixes: f102d66b335a ("netfilter: nf_tables: use dedicated mutex to guard transactions")

Without it, stable trees that carry f102d66b335a may not pick up this fix.
Other patches in the same series do have Fixes: tags, for example
"netfilter: conntrack: fix unlikely UaF in gc worker" (also an LLM finding
from the same reporter) and "netfilter: nft_compat: restrict raw table
targets/matches".

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org

Re: [PATCH net-next 09/15] netfilter: nft_exthdr: avoid 60 bytes stack buffer in TCP option mangling

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message says "This patch also updates the logic to
  drop malformed packets, before this patch NFT_BREAK was used as a…

--

netfilter: nft_exthdr: avoid 60 bytes stack buffer in TCP option mangling

This commit changes nft_exthdr_tcp_set_eval() and
nft_exthdr_tcp_strip_eval() so they no longer call
nft_tcp_header_pointer(). Each function now reads only the 20 byte fixed
TCP header with skb_header_pointer(), validates doff, and leaves it to
skb_ensure_writable() to pull the options. This removes a 60 byte stack
buffer and a redundant second header copy from both functions.

> diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
> index 3492426dccf7f..861bab93f4352 100644
> --- a/net/netfilter/nft_exthdr.c
> +++ b/net/netfilter/nft_exthdr.c
> @@ -233,16 +233,23 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,

[ ... ]

> -	tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
> +	if (pkt->tprot != IPPROTO_TCP || pkt->fragoff)
> +		goto err;
> +
> +	tcph = skb_header_pointer(pkt->skb, nft_thoff(pkt), sizeof(_tcph), &_tcph);
>  	if (!tcph)
>  		goto err;
>  
> +	tcphdr_len = __tcp_hdrlen(tcph);
> +	if (tcphdr_len < sizeof(*tcph) ||
> +	    tcphdr_len > sizeof(*tcph) + MAX_TCP_OPTION_SPACE)
> +		goto err;
> +
>  	if (skb_ensure_writable(pkt->skb, nft_thoff(pkt) + tcphdr_len))
>  		goto err;
>  
> @@ -313,16 +320,23 @@ static void nft_exthdr_tcp_strip_eval(const struct nft_expr *expr,

[ ... ]

> -	tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
> +	if (pkt->tprot != IPPROTO_TCP || pkt->fragoff)
> +		goto err;
> +
> +	tcph = skb_header_pointer(pkt->skb, nft_thoff(pkt), sizeof(_tcph), &_tcph);
>  	if (!tcph)
>  		goto err;
>  
> +	tcphdr_len = __tcp_hdrlen(tcph);
> +	if (tcphdr_len < sizeof(*tcph) ||
> +	    tcphdr_len > sizeof(*tcph) + MAX_TCP_OPTION_SPACE)
> +		goto err;
> +
>  	if (skb_ensure_writable(pkt->skb, nft_thoff(pkt) + tcphdr_len))
>  		goto drop;

[Severity: Medium]

The commit message says:

  This patch also updates the logic to drop malformed packets, before this
  patch NFT_BREAK was used as a verdict instead.

Does that match what the diff does? The diff does not touch any verdict
code. The change in behaviour only comes from removing the second,
full-length skb_header_pointer() call that nft_tcp_header_pointer() made:

  return skb_header_pointer(pkt->skb, nft_thoff(pkt), *tcphdr_len, buffer);

Before this patch, a truncated TCP packet (thoff + doff * 4 > skb->len)
made that call return NULL. Both functions then took goto err and
returned NFT_BREAK.

With this patch the 20 byte read succeeds and the doff check passes. The
packet then fails in skb_ensure_writable()->pskb_may_pull(), and the
result differs per function:

  nft_exthdr_tcp_strip_eval(): goto drop -> NF_DROP (new)
  nft_exthdr_tcp_set_eval():   goto err  -> NFT_BREAK (unchanged)

A packet with doff < 5 still takes goto err and gets NFT_BREAK in both
functions.

As a result, a tcp option reset rule now drops a truncated packet, while
a tcp option ... set rule treats the same packet as a non-match.

Should the drop policy also cover nft_exthdr_tcp_set_eval() and the
doff < 5 case? If not, could the commit message say that the change only
affects truncated options in nft_exthdr_tcp_strip_eval()?

Separately, this isn't a bug, but the new upper bound check:

    tcphdr_len > sizeof(*tcph) + MAX_TCP_OPTION_SPACE

can never be true. doff is a 4 bit field, so tcphdr_len is at most 60.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org

Re: [PATCH net-next 11/15] netfilter: nft_compat: restrict raw table targets/matches

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The new 'raw' check in nft_compat_chain_validate_dependency()
  (net/netfilter/nft_compat.c:61) depends on basechain->ops.priority.
- [Low] The new comment in nft_compat_chain_validate_dependency() says
  the legacy raw table 'only ever has prerouting/output chains at this…

--

netfilter: nft_compat: restrict raw table targets/matches

This patch extends nft_compat_chain_validate_dependency() so that xt
targets and matches that declare .table = "raw" (CT, NOTRACK) are only
accepted in ip/ip6 base chains hooked at prerouting or output with
priority NF_IP_PRI_RAW. The goal is to stop the CT target from being
loaded at a hook or priority where it would run after conntrack.

> diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
> index 63864b9282590..88ddaf47f5879 100644
> --- a/net/netfilter/nft_compat.c
> +++ b/net/netfilter/nft_compat.c

[ ... ]

> @@ -45,6 +46,20 @@ static int nft_compat_chain_validate_dependency(const struct nft_ctx *ctx,
>  			type = NFT_CHAIN_T_NAT;
>  		if (basechain->type->type != type)
>  			return -EINVAL;
> +	} else if (strcmp(tablename, "raw") == 0) {
> +		/* iptables-nft emulates the legacy raw table, which only
> +		 * ever has prerouting/output chains at this exact priority.
> +		 */

[Severity: Low]
Is this comment accurate? The legacy raw table can also be registered at a
second priority. In net/ipv4/netfilter/iptable_raw.c:

static const struct xt_table packet_raw_before_defrag = {
	.name = "raw",
	.valid_hooks =  RAW_VALID_HOOKS,
	.me = THIS_MODULE,
	.af = NFPROTO_IPV4,
	.priority = NF_IP_PRI_RAW_BEFORE_DEFRAG,
};

The table is selected when the raw_before_defrag module parameter is set.
ip6table_raw.c does the same with NF_IP6_PRI_RAW_BEFORE_DEFRAG.

A CT/NOTRACK target at -450 still runs ahead of conntrack. The new check
below accepts only NF_IP_PRI_RAW, though, so it rejects that priority with
-EINVAL. That makes it stricter than "what xtables enforces", as the
commit message puts it.

Should the comment say that iptables-nft always uses -300? Alternatively,
the check could also accept the before-defrag priority, or any priority
below NF_IP_PRI_CONNTRACK.

> +		if (ctx->family != NFPROTO_IPV4 &&
> +		    ctx->family != NFPROTO_IPV6)
> +			return -EINVAL;
> +
> +		if (basechain->ops.hooknum != NF_INET_PRE_ROUTING &&
> +		    basechain->ops.hooknum != NF_INET_LOCAL_OUT)
> +			return -EINVAL;
> +
> +		if (basechain->ops.priority != NF_IP_PRI_RAW)
> +			return -EINVAL;

[Severity: High]
Can the nf_tables chain validation cache let a jumped-to chain skip this
priority check?

nft_chain_vstate_valid() treats a regular chain as already validated using
only the calling base chain's type, hooknum and the depth:

net/netfilter/nf_tables_api.c:nft_chain_vstate_valid() {
    ...
	if (chain->vstate.depth >= ctx->level &&
	    chain->vstate.hook_mask[type] & BIT(hooknum))
		return true;
    ...
}

nft_chain_vstate_update() records only hook_mask[type] and depth, never the
base chain priority. nft_table_validate() clears vstate only after it has
walked every base chain, so cached results carry over from one base chain
to the next.

Take an ip table named "raw" with:

  base chain A: type filter, hook prerouting, priority -300, rule "jump X"
  base chain B: type filter, hook prerouting, priority 0, rule "jump X"
  regular chain X: an nft_compat rule using the CT target

Chain A is created first. Validating A goes like this:

nft_table_validate()
  nft_chain_validate(A)
    nft_immediate_validate()            /* level 1 */
      nft_chain_validate(X)
        nft_target_validate()
          nft_compat_chain_validate_dependency()  /* passes, A is -300 */
        nft_chain_vstate_update()       /* FILTER/PRE_ROUTING, depth 1 */

Then for B:

  nft_chain_validate(B)
    nft_immediate_validate()            /* level 1 */
      nft_chain_validate(X)
        nft_chain_vstate_valid()        /* true */
        return 0;

So the new check never runs with B as ctx->chain, and the ruleset is
accepted.

At runtime, nf_conntrack_in() can leave skb->_nfct == 0 for an invalid
packet. It also clears any raw-table template before tracking, so an
unconditional jump in A doesn't help. B then jumps to X after conntrack,
and xt_ct_target() attaches the template:

	if (skb->_nfct != 0)
		return XT_CONTINUE;

	if (ct) {
		refcount_inc(&ct->ct_general.use);
		nf_ct_set(skb, ct, IP_CT_NEW);

Isn't this the same "after the real conntrack hook has already run" case
that the commit message says this patch closes? It can be reached with
CAP_NET_ADMIN in an unprivileged user namespace.

Could the vstate key be made priority-aware (or carry a pre-conntrack
bit)? Other options would be to skip the cache for chains that hold compat
expressions with a table dependency, or to run this check for every base
chain that can reach the rule.

>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org

Re: [PATCH net-next 12/15] netfilter: flowtable: set inner protocol when popping tunnel header

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message says the patch makes nf_flow_ip_tunnel_pop()
  pop the outer header and set skb->protocol "instead of bailing out…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but
  nf_flow_ip_tunnel_pop() strips the outer tunnel header with plain…

--

netfilter: flowtable: set inner protocol when popping tunnel header

This commit makes nf_flow_ip_tunnel_pop() set skb->protocol from
ctx->tun.inner_proto after it pulls the outer tunnel header. The commit
message calls it a preliminary patch for IPv4 over IPv6 and SIT tunnel
flowtable offload.

> Pop the outer tunnel header and set skb->protocol to the inner protocol
> derived from ctx->tun.inner_proto in nf_flow_ip_tunnel_pop(), instead of
> bailing out unless the tunnel carries IP-in-IP or IPv6-in-IPv6.

[Severity: Low]
Is this description accurate? The early return at the top of
nf_flow_ip_tunnel_pop() is unchanged by this patch:

	if (ctx->tun.inner_proto != IPPROTO_IPIP &&
	    ctx->tun.inner_proto != IPPROTO_IPV6)
		return;

The only change is the new skb->protocol assignment.

With the current parsers, that assignment also looks like a no-op:

- nf_flow_ip4_tunnel_proto() sets inner_proto to IPPROTO_IPIP only when
  the outer header is IPv4.
- nf_flow_ip6_tunnel_proto() sets IPPROTO_IPV6 only when the outer header
  is IPv6.
- nf_flow_encap_pop() calls the pop only when skb->protocol is already
  the outer family:

	if (skb->protocol == htons(ETH_P_IP) ||
	    skb->protocol == htons(ETH_P_IPV6))
		nf_flow_ip_tunnel_pop(ctx, skb);

So the value written always matches what skb->protocol already holds.
The rest of the series, up to "netfilter: flowtable: unify tunnel push for
IPv4 and IPv6", does not change the parsers or this guard.

Could the commit message say that the guard is kept? It could also say
that the assignment only has an effect once cross-family parsing
(IPv4-over-IPv6/SIT) is added.

Removing the guard, as the message describes, would not be correct.
Non-tunnel flows have inner_proto == 0, so they would get skb->protocol
set to ETH_P_IPV6.

> diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
> index c8c29a9a16844..29d1a9f4a362b 100644
> --- a/net/netfilter/nf_flow_table_ip.c
> +++ b/net/netfilter/nf_flow_table_ip.c
> @@ -372,6 +372,11 @@ static void nf_flow_ip_tunnel_pop(struct nf_flowtable_ctx *ctx,
>  
>  	skb_pull(skb, ctx->tun.hdr_size);
>  	skb_reset_network_header(skb);
> +
> +	if (ctx->tun.inner_proto == IPPROTO_IPIP)
> +		skb->protocol = htons(ETH_P_IP);
> +	else
> +		skb->protocol = htons(ETH_P_IPV6);

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. Should
the skb_pull() above be skb_pull_rcsum()?

The VLAN and PPPoE cases in nf_flow_encap_pop() in the same file use
skb_pull_rcsum(). __iptunnel_pull_header() also adjusts the checksum
before it sets the inner protocol:

net/ipv4/ip_tunnel_core.c:__iptunnel_pull_header() {
    ...
	skb_pull_rcsum(skb, hdr_len);
    ...
		skb->protocol = inner_proto;
    ...
}

This patch adds the skb->protocol step from that pattern, but not the
checksum adjustment.

Take a CHECKSUM_COMPLETE skb with an outer IPv6 header (ip6ip6 today,
IPv4-over-IPv6 later). After the pull, skb->csum still includes the 40
bytes of the removed header. An outer IPv4 header does not cause this,
because its valid header checksum makes the header's ones'-complement
sum effectively zero.

Suppose such a forwarded skb is looped back into a receive path. One
example is veth into another netns, where __dev_forward_skb2() keeps
CHECKSUM_COMPLETE. Would the L4 checksum check then fail on the stale
skb->csum? The stack would recompute the checksum in software and call
netdev_rx_csum_fault(), which prints a rate-limited warning.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org

Re: [PATCH net-next 02/15] selftests: netfilter: nft_queue.sh: only queue icmp echo request/reply

[Florian Westphal]

netdev-bot+sashiko@kernel.org <netdev-bot+sashiko@kernel.org> wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
> 
> Low priority / nit picks for your consideration:
> 
> - [Low] The commit fixes a reproducible flake in the bridge NFQUEUE
>   selftest (about one failure a day in NIPA), but its message has no…
> 
> --
> 
> selftests: netfilter: nft_queue.sh: only queue icmp echo request/reply
> 
> The patch changes the ipv4 nfq rule in load_ruleset() so it only queues
> icmp echo request/reply, which the icmpv6 rule already does. ICMP time
> exceeded messages left over after the stress test no longer inflate the
> bridge queue packet counts.
> 
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > Reviewed-by: Florian Westphal <fw@strlen.de>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> [Severity: Low]
> This isn't a bug, but should this have a Fixes: tag?

Not really.  Was only ever caught by netdev CI which already
carries this fix locally.

> The flake looks like it started with:
> 
> commit 8a2cfe7951f6 ("selftests: nft_queue.sh: add a bridge queue test")

Yes.

> Would something like this be worth adding?
> 
> Fixes: 8a2cfe7951f6 ("selftests: nft_queue.sh: add a bridge queue test")

I'm not sure its worth doing.

Re: [PATCH net-next 04/15] netfilter: nf_queue: limit the hook drop flush to the unregistered hook point

[Florian Westphal]

netdev-bot+sashiko@kernel.org <netdev-bot+sashiko@kernel.org> wrote:

@Antonio: TL;DR: if Pablo wants a v2, please send a new patch that adds
the needed nf_queue_nf_hook_drop() calls in brnf and conntrack.

Otherwise, please followup after this hits net-next tree.

> - [High] Use-after-free of module code: after this patch, `rmmod
>   br_netfilter` no longer drops queued packets that point into…
> - [Medium] Conntrack module unload can now stall on queued packets, and
>   the commit message does not mention it.
> - [Low] The commit message has no Fixes: tag.
> 
> --
> 
> [Severity: Low]
> Should this have a Fixes: tag?

Absolutely NOT.

> Without the trailer, stable maintainers may miss this fix. The message
> also does not say the omission is deliberate.

This is NOT a fix.  And the problems highlight that this belongs
in -next and not in stable trees.

> [Severity: High]
> After this change, can packets whose okfn points into br_netfilter.ko stay
> queued after that module is unloaded?

Yes, br nf strikes again.

> Is a flush-all, such as nf_queue_nf_hook_drop(net, NULL), needed in the
> br_netfilter exit path, or some other flush at module teardown for callers
> whose okfn is module code?

Yes, needs to be added for brnf.  Could be handled as a followup,
though, ample of time to fix this until this gets into a non-rc kernel.

> [Severity: Medium]
> Could rmmod nf_conntrack now block while userspace holds verdicts for
> queued packets?
>
> Before this patch, that unregistration flushed every queued entry. This
> released the nf_conn references held through skb->_nfct.
> 
> This runs while pernet_ops_rwsem is held for write. Netns creation and
> teardown would stall until a verdict is sent or the queue socket is
> closed.
> 
> The commit message says nf_ct_iterate_destroy() keeps the NULL flush on
> module exit. That path covers helpers and NAT, not nf_conntrack itself,
> and the message does not mention this case.

> Should nf_conntrack's exit path also flush with NULL?

Yes, nf_conntrack_fini_net() would be a candiate.

But unlike the brnf case I think this one is relatively harmless.

If v2 wanted: add

Re: [PATCH net-next 05/15] selftests: netfilter: nft_queue: check the scope of the hook drop flush

[Florian Westphal]

netdev-bot+sashiko@kernel.org <netdev-bot+sashiko@kernel.org> wrote:
> - [Medium] The new test's failure does not reach the script's exit
>   status.
> - [Low] The 'keep' case cannot tell the two halves of nf_hook_cmp()
>   apart.
> - [Low] In hook_drop_case() the exit status of the command under test,
>   `ip netns exec "$ns1" nft "$@"` (line 932), is never checked, and the…
> - [Low] hook_drop_case() ignores the return value of `busywait
>   "$BUSYWAIT_TIMEOUT" hook_drop_all_queued "$packets"` (line 930).

I'll do a full quote here, despite not really responding: Antonio, if
you have to send a v2 for the nfnetlink_drop change, then please
consider sending a v2 for this one too. Otherwise I'm not sure its worth
following up with this one.

> selftests: netfilter: nft_queue: check the scope of the hook drop flush
> 
> This adds test_hook_drop_scope to nft_queue.sh. The test queues 20 UDP
> packets to the nf_queue helper, which holds the verdicts, and deletes a
> base chain while the packets wait. It then checks whether the packets
> were kept or dropped. It also adds a -H option to nf_queue that holds
> verdicts until SIGUSR1 is received.
> 
> > diff --git a/tools/testing/selftests/net/netfilter/nft_queue.sh b/tools/testing/selftests/net/netfilter/nft_queue.sh
> > index c8d1f2eb41339..a07652813a4b0 100755
> > --- a/tools/testing/selftests/net/netfilter/nft_queue.sh
> > +++ b/tools/testing/selftests/net/netfilter/nft_queue.sh
> 
> [ ... ]
> 
> > +hook_drop_case()
> > +{
> 
> [ ... ]
> 
> > +	ip netns exec "$ns1" bash -c \
> > +		"for i in \$(seq $packets); do echo x > /dev/udp/127.0.0.1/12345; done"
> > +
> > +	busywait "$BUSYWAIT_TIMEOUT" hook_drop_all_queued "$packets"
> 
> [Severity: Low]
> What happens if this busywait times out? On timeout, loopy_wait() in
> lib.sh just returns 1. It does not abort the caller:
> 
> tools/testing/selftests/net/lib.sh:loopy_wait() {
>     ...
> 		if ((current_time - start_time > timeout_ms)); then
> 			echo -n "$out"
> 			return 1
> 		fi
>     ...
> }
> 
> The return value is ignored here. So the test still deletes the chain
> and classifies the result when fewer than 20 packets were queued, for
> example when some of the sends were lost before they reached the queue.
> 
> The drop classification further down only checks for zero:
> 
> 	elif [ "$queued" = "0" ] && [ "$delivered" = "0" ]; then
> 		result="drop"
> 
> Could the drop case then report PASS with only part of the batch queued,
> or with nothing queued at all? In the keep case the same situation
> would be reported as inconsistent.
> 
> > +
> > +	ip netns exec "$ns1" nft "$@"
> > +	queued=$(hook_drop_queued)
> 
> [Severity: Low]
> Should the exit status of this nft command be checked?
> 
> Suppose the deletion fails, or test_hook_drop_scope() passes the wrong
> arguments. Then no hook is unregistered and the 20 packets stay queued.
> SIGUSR1 releases them, the input counter reaches 20 and nf_queue prints
> "20 packets total". The keep case would report PASS even though no
> chain was deleted.
> 
> Other deletions in this file do check the result, for example in
> test_udp_gro_ct():
> 
> 	if ! ip netns exec "$nsrouter" nft delete table inet udpq; then
> 		echo "FAIL: Could not delete udpq table"
> 		ret=1
> 
> [ ... ]
> 
> > +test_hook_drop_scope()
> > +{
> > +	# the hook array of another hook point changes, queued packets stay
> > +	hook_drop_case keep "base chain removed at another hook point" \
> > +		delete chain ip6 unrelated prerouting
> 
> [Severity: Low]
> Does this keep case test the hook number check and the family check in
> nf_hook_cmp() separately? The queued packets are IPv4 (sent to
> 127.0.0.1) at NF_INET_LOCAL_OUT. The deleted chain differs from them in
> both family (ip6) and hook (prerouting):
> 
> net/netfilter/nfnetlink_queue.c:nf_hook_cmp() {
>     ...
> 	if (entry->state.hook != hooknum)
> 		return 0;
> 
> 	if (pf == NFPROTO_INET)
> 		return entry->state.pf == NFPROTO_IPV4 ||
> 		       entry->state.pf == NFPROTO_IPV6;
> 
> 	return entry->state.pf == pf;
> }
> 
> If nf_hook_cmp() compared only the hook number, or only the family, it
> would still keep the entries and this case would pass. The drop case
> matches on both fields, so it doesn't isolate either check.
> 
> Would it make sense to add a chain at the same hook in another family
> (for example ip6 output)? A chain at another hook in the same family
> (for example ip prerouting) would cover the other check.
> 
> The testing notes of the previous patch, "netfilter: nf_queue: limit the
> hook drop flush to the unregistered hook point", say that deleting an
> ip6 table with an output base chain does not drop the packets. That
> case isn't encoded in this test.
> 
> [ ... ]
> 
> > @@ -909,6 +1016,7 @@ test_queue_stress
> >  # should be last, adds vrf device in ns1 and changes routes
> >  test_icmp_vrf
> >  test_queue_removal
> > +test_hook_drop_scope
> >  
> >  # turns router into a bridge
> >  test_queue_bridge
> 
> [Severity: Medium]
> Does a failure in test_hook_drop_scope() reach the exit status of the
> script? hook_drop_case() records a failure only by setting the global
> ret=1.
> 
> test_queue_bridge runs afterwards and calls test_queue() twice.
> test_queue() does this:
> 
> tools/testing/selftests/net/netfilter/nft_queue.sh:test_queue() {
>     ...
> 	wait
> 	ret=$?
>     ...
> }
> 
> A bare wait returns 0, so ret is reset to 0 whenever the bridge tests
> pass. The script then reports success with exit $ret.
> 
> The FAIL line still goes to stderr, but kselftest runners decide pass
> or fail from the exit code. The same reset already affects the earlier
> tests in this sequence.
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org
> 

Re: [PATCH net-next 07/15] netfilter: nf_tables: switch chain_types array to rcu

[Florian Westphal]

netdev-bot+sashiko@kernel.org <netdev-bot+sashiko@kernel.org> wrote:
> Low priority / nit picks for your consideration:
> 
> - [Low] Commit "netfilter: nf_tables: switch chain_types array to rcu"
>   fixes a module-memory use-after-free but has no Fixes: tag and no Cc:…
> 
> --
> 
> netfilter: nf_tables: switch chain_types array to rcu
> 
> This converts the global chain_type array in nf_tables_api.c to RCU and
> takes the module reference on the chain type owner inside the RCU read
> section. That closes a window where a transaction could dereference an
> nft_chain_type freed by a concurrent rmmod of nft_chain_nat, and
> nft_chain_nat_exit() now calls synchronize_rcu() after unregistering.
> 
> > Reported-by: Vega <vega@nebusec.ai>
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> [Severity: Low]
> Should this patch carry a Fixes: tag?

No need.  This is an LLM report, not a real bug ever encountered.

Would need busyloop doing rmmod on nft modules with nft transactions happening
in parallel, this is not a realistic enough scenario to entertain a
Fixes tag IMO.

> Fixes: f102d66b335a ("netfilter: nf_tables: use dedicated mutex to guard transactions")

Looks correct, but I don't think this is worth placing in -stable.
Even if AI says the patch is fine and correct, we've had enough cases of
AI missing things (resp. humans missing bogosity in AI generated patches)
that we should make "AI is fine with it" the new "it compiles, ship it".

> Without it, stable trees that carry f102d66b335a may not pick up this fix.
> Other patches in the same series do have Fixes: tags, for example
> "netfilter: conntrack: fix unlikely UaF in gc worker" (also an LLM finding
> from the same reporter) and "netfilter: nft_compat: restrict raw table
> targets/matches".

Its a tradeoff.  This is fix is neither obviously simple, nor the bug
severe or frequent enough.  The raw table thing e.g. allows deliberate
kernel crash for unpriv netns, but this one needs full rmmod capability.

Re: [PATCH net-next 11/15] netfilter: nft_compat: restrict raw table targets/matches

[Florian Westphal]

netdev-bot+sashiko@kernel.org <netdev-bot+sashiko@kernel.org> wrote:

TL;DR, this patch is fine as-is, but a complementary patch to
validate nft_ct.c and handle vstate cache is missing from this PR.  This
does not cause any breakage, though.

> - [High] The new 'raw' check in nft_compat_chain_validate_dependency()
>   (net/netfilter/nft_compat.c:61) depends on basechain->ops.priority.
> - [Low] The new comment in nft_compat_chain_validate_dependency() says
>   the legacy raw table 'only ever has prerouting/output chains at this…
> > @@ -45,6 +46,20 @@ static int nft_compat_chain_validate_dependency(const struct nft_ctx *ctx,
> >  			type = NFT_CHAIN_T_NAT;
> >  		if (basechain->type->type != type)
> >  			return -EINVAL;
> > +	} else if (strcmp(tablename, "raw") == 0) {
> > +		/* iptables-nft emulates the legacy raw table, which only
> > +		 * ever has prerouting/output chains at this exact priority.
> > +		 */
> 
> [Severity: Low]
> Is this comment accurate?

Is this a trick question?

> The legacy raw table can also be registered at a
> second priority. In net/ipv4/netfilter/iptable_raw.c:

[..]

> The table is selected when the raw_before_defrag module parameter is set.
> ip6table_raw.c does the same with NF_IP6_PRI_RAW_BEFORE_DEFRAG.

iptables-nft doesn't use iptable_raw.c nor does it emulate this
module param.

> A CT/NOTRACK target at -450 still runs ahead of conntrack. The new check
> below accepts only NF_IP_PRI_RAW, though, so it rejects that priority with
> -EINVAL. That makes it stricter than "what xtables enforces", as the
> commit message puts it.

Why should I support -450 ? iptables-nft doesn't use/support that.

> Should the comment say that iptables-nft always uses -300?

*shrug*, thats what I intended to convey: iptables-nft only uses
-300, hence hard refusal.

> Alternatively,
> the check could also accept the before-defrag priority, or any priority
> below NF_IP_PRI_CONNTRACK.

"Yes" (but I don't see why it should)

> [Severity: High]
> Can the nf_tables chain validation cache let a jumped-to chain skip this
> priority check?

Yes, this patch only works with this applied as well:

[PATCH nf-next] netfilter: nft_ct: validate hook and priority for ct zone set
https://lore.kernel.org/netfilter-devel/atDHof0kFWIWIf6-@strlen.de/T/#t

> nft_chain_vstate_valid() treats a regular chain as already validated using
> only the calling base chain's type, hooknum and the depth:
> 
> net/netfilter/nf_tables_api.c:nft_chain_vstate_valid() {
>     ...
> 	if (chain->vstate.depth >= ctx->level &&
> 	    chain->vstate.hook_mask[type] & BIT(hooknum))
> 		return true;
>     ...
> }
> 
> nft_chain_vstate_update() records only hook_mask[type] and depth, never the
> base chain priority. nft_table_validate() clears vstate only after it has
> walked every base chain, so cached results carry over from one base chain
> to the next.

Yes, the above linked patch did add two prio fields to the vstate for this.

>   base chain A: type filter, hook prerouting, priority -300, rule "jump X"
>   base chain B: type filter, hook prerouting, priority 0, rule "jump X"
>   regular chain X: an nft_compat rule using the CT target
> 
> Chain A is created first. Validating A goes like this:
> 
> nft_table_validate()
>   nft_chain_validate(A)
>     nft_immediate_validate()            /* level 1 */
>       nft_chain_validate(X)
>         nft_target_validate()
>           nft_compat_chain_validate_dependency()  /* passes, A is -300 */
>         nft_chain_vstate_update()       /* FILTER/PRE_ROUTING, depth 1 */

With the vstate patch, this should now have vstate change set to -300
for min and max priorities.

> Then for B:
> 
>   nft_chain_validate(B)
>     nft_immediate_validate()            /* level 1 */
>       nft_chain_validate(X)
>         nft_chain_vstate_valid()        /* true */
>         return 0;
> 
> So the new check never runs with B as ctx->chain, and the ruleset is
> accepted.

With vstate patch, this would fail the vstate_valid test because prio 0
is larger than previous prio_max validation (-300).

> At runtime, nf_conntrack_in() can leave skb->_nfct == 0 for an invalid
> packet. It also clears any raw-table template before tracking, so an
> unconditional jump in A doesn't help. B then jumps to X after conntrack,
> and xt_ct_target() attaches the template:
> 
> 	if (skb->_nfct != 0)
> 		return XT_CONTINUE;
> 
> 	if (ct) {
> 		refcount_inc(&ct->ct_general.use);
> 		nf_ct_set(skb, ct, IP_CT_NEW);
> 
> Isn't this the same "after the real conntrack hook has already run" case
> that the commit message says this patch closes? It can be reached with
> CAP_NET_ADMIN in an unprivileged user namespace.

Yes.

> Could the vstate key be made priority-aware (or carry a pre-conntrack
> bit)?

Yes, patch is on ML.  But even if I had placed the vstate patch in this
one, its still incomplete because nft_ct allows template attachmnent as
well.

Both patches are needed.

> Other options would be to skip the cache for chains that hold compat
> expressions with a table dependency, or to run this check for every base
> chain that can reach the rule.

Would work if nft_ct.c would not exist.

Re: [PATCH net-next 09/15] netfilter: nft_exthdr: avoid 60 bytes stack buffer in TCP option mangling

[Pablo Neira Ayuso]

On Mon, Oct 19, 2026 at 01:21:26AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] The commit message says "This patch also updates the logic to
>   drop malformed packets, before this patch NFT_BREAK was used as a…
> 
> --
> 
> netfilter: nft_exthdr: avoid 60 bytes stack buffer in TCP option mangling
> 
> This commit changes nft_exthdr_tcp_set_eval() and
> nft_exthdr_tcp_strip_eval() so they no longer call
> nft_tcp_header_pointer(). Each function now reads only the 20 byte fixed
> TCP header with skb_header_pointer(), validates doff, and leaves it to
> skb_ensure_writable() to pull the options. This removes a 60 byte stack
> buffer and a redundant second header copy from both functions.
> 
> > diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
> > index 3492426dccf7f..861bab93f4352 100644
> > --- a/net/netfilter/nft_exthdr.c
> > +++ b/net/netfilter/nft_exthdr.c
> > @@ -233,16 +233,23 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
> 
> [ ... ]
> 
> > -	tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
> > +	if (pkt->tprot != IPPROTO_TCP || pkt->fragoff)
> > +		goto err;
> > +
> > +	tcph = skb_header_pointer(pkt->skb, nft_thoff(pkt), sizeof(_tcph), &_tcph);
> >  	if (!tcph)
> >  		goto err;
> >  
> > +	tcphdr_len = __tcp_hdrlen(tcph);
> > +	if (tcphdr_len < sizeof(*tcph) ||
> > +	    tcphdr_len > sizeof(*tcph) + MAX_TCP_OPTION_SPACE)
> > +		goto err;
> > +
> >  	if (skb_ensure_writable(pkt->skb, nft_thoff(pkt) + tcphdr_len))
> >  		goto err;
> >  
> > @@ -313,16 +320,23 @@ static void nft_exthdr_tcp_strip_eval(const struct nft_expr *expr,
> 
> [ ... ]
> 
> > -	tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
> > +	if (pkt->tprot != IPPROTO_TCP || pkt->fragoff)
> > +		goto err;
> > +
> > +	tcph = skb_header_pointer(pkt->skb, nft_thoff(pkt), sizeof(_tcph), &_tcph);
> >  	if (!tcph)
> >  		goto err;
> >  
> > +	tcphdr_len = __tcp_hdrlen(tcph);
> > +	if (tcphdr_len < sizeof(*tcph) ||
> > +	    tcphdr_len > sizeof(*tcph) + MAX_TCP_OPTION_SPACE)
> > +		goto err;
> > +
> >  	if (skb_ensure_writable(pkt->skb, nft_thoff(pkt) + tcphdr_len))
> >  		goto drop;
> 
> [Severity: Medium]

Nothing wrong in this patch.

> The commit message says:
> 
>   This patch also updates the logic to drop malformed packets, before this
>   patch NFT_BREAK was used as a verdict instead.

I added this comment to make this AI robot and it keeps complaining...

> Does that match what the diff does? The diff does not touch any verdict
> code. The change in behaviour only comes from removing the second,
> full-length skb_header_pointer() call that nft_tcp_header_pointer() made:
> 
>   return skb_header_pointer(pkt->skb, nft_thoff(pkt), *tcphdr_len, buffer);
> 
> Before this patch, a truncated TCP packet (thoff + doff * 4 > skb->len)
> made that call return NULL. Both functions then took goto err and
> returned NFT_BREAK.

Yes, this is what the comment aims to say.

Re: [PATCH net-next 12/15] netfilter: flowtable: set inner protocol when popping tunnel header

[Pablo Neira Ayuso]

On Mon, Oct 19, 2026 at 01:21:28AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
> 
> Low priority / nit picks for your consideration:
> 
> - [Low] The commit message says the patch makes nf_flow_ip_tunnel_pop()
>   pop the outer header and set skb->protocol "instead of bailing out…
> 
> Pre-existing issues:
> - [Medium] This problem wasn't introduced by this patch, but
>   nf_flow_ip_tunnel_pop() strips the outer tunnel header with plain…
> 
> --
> 
> netfilter: flowtable: set inner protocol when popping tunnel header
> 
> This commit makes nf_flow_ip_tunnel_pop() set skb->protocol from
> ctx->tun.inner_proto after it pulls the outer tunnel header. The commit
> message calls it a preliminary patch for IPv4 over IPv6 and SIT tunnel
> flowtable offload.
> 
> > Pop the outer tunnel header and set skb->protocol to the inner protocol
> > derived from ctx->tun.inner_proto in nf_flow_ip_tunnel_pop(), instead of
> > bailing out unless the tunnel carries IP-in-IP or IPv6-in-IPv6.
> 
> [Severity: Low]
> Is this description accurate?

I think description is good enough, this is pedantic.

Re: [PATCH net-next 02/15] selftests: netfilter: nft_queue.sh: only queue icmp echo request/reply

[Pablo Neira Ayuso]

On Mon, Oct 19, 2026 at 01:21:21AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
> 
> Low priority / nit picks for your consideration:
> 
> - [Low] The commit fixes a reproducible flake in the bridge NFQUEUE
>   selftest (about one failure a day in NIPA), but its message has no…
> 
> --
> 
> selftests: netfilter: nft_queue.sh: only queue icmp echo request/reply
> 
> The patch changes the ipv4 nfq rule in load_ruleset() so it only queues
> icmp echo request/reply, which the icmpv6 rule already does. ICMP time
> exceeded messages left over after the stress test no longer inflate the
> bridge queue packet counts.
> 
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > Reviewed-by: Florian Westphal <fw@strlen.de>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> [Severity: Low]
> This isn't a bug, but should this have a Fixes: tag?

This is nf-next material, no.

Re: [PATCH net-next 01/15] netfilter: disable br_netfilter in user namespaces

[Pablo Neira Ayuso]

On Mon, Oct 19, 2026 at 01:21:20AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] tools/testing/selftests/net/netfilter/br_netfilter_queue.sh
>   re-execs itself under `unshare -n` (line 40) and then runs `sysctl…
> 
> --
> 
> netfilter: disable br_netfilter in user namespaces
> 
> This commit disables br_netfilter in network namespaces that are not
> owned by init_user_ns. In those namespaces it skips registration of the
> bridge-nf sysctls and of the br_netfilter hooks. It also adds a boot-time
> warning that the module is slated for removal in 2027, and updates the
> bridge documentation to match.
> 
> > diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
> > index 0a394e5f43916..e15355ff215a7 100644
> > --- a/net/bridge/br_netfilter_hooks.c
> > +++ b/net/bridge/br_netfilter_hooks.c
> 
> [ ... ]
> 
> >  static int __net_init brnf_init_net(struct net *net)
> >  {
> > +	if (net->user_ns != &init_user_ns) {
> > +		struct brnf_net *brnet = net_generic(net, brnf_net_id);
> > +
> > +		brnet->in_userns = true;
> > +		pr_warn_once("br_netfilter no longer supported in user namespaces\n");
> > +		return 0;
> > +	}
> > +
> >  	return br_netfilter_sysctl_init_net(net);
> >  }
> 
> [Severity: Medium]
> Should tools/testing/selftests/net/netfilter/br_netfilter_queue.sh be
> updated along with this change?
> 
> That script re-execs itself in a new netns via unshare -n. The new netns
> inherits the caller's user namespace:
> 
>     [ "${1}" != "run" ] && { unshare -n "${0}" run; exit $?; }
> 
> It then treats a missing sysctl as a hard failure:
> 
>     modprobe -q br_netfilter
> 
>     sysctl net.bridge.bridge-nf-call-iptables=1 || exit 1
> 
> Suppose the selftests run inside a non-initial user namespace, for
> example a rootless container or a CI job under unshare -r.

This patch is disabling br_netfilter for userns, so it is normal to me
that such script run under these assumptions do not work anymore.

Re: [PATCH net-next 04/15] netfilter: nf_queue: limit the hook drop flush to the unregistered hook point

[Pablo Neira Ayuso]

On Mon, Oct 19, 2026 at 06:49:47AM +0200, Florian Westphal wrote:
> netdev-bot+sashiko@kernel.org <netdev-bot+sashiko@kernel.org> wrote:
> 
> @Antonio: TL;DR: if Pablo wants a v2, please send a new patch that adds
> the needed nf_queue_nf_hook_drop() calls in brnf and conntrack.

I'd prefer a follow up once this PR is pulled in.

All the LLM generated comments I'm reading seem weak to me.

[...]
> > Should nf_conntrack's exit path also flush with NULL?
> 
> Yes, nf_conntrack_fini_net() would be a candiate.

Sound like a pre-existing issue.

> But unlike the brnf case I think this one is relatively harmless.

That needs a follow up, I would suggest.