netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net,v2 0/8] Netfilter/IPVS fixes for net
@ 2026-09-18 11:28 Pablo Neira Ayuso
  2026-09-18 11:28 ` [PATCH net 1/8] netfilter: flowtable: publish HW_DEAD after worker is done Pablo Neira Ayuso
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-18 11:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet, horms, fw, ja

v2: - keep back IPVS patch
      "ipvs: filter some flags received in the backup server"
      so Julian has a change to revisit the commit description.
    - Add check for dead bit in nf_tables catchall patch.

-o-

Hi,

The following patchset contains Netfilter/IPVS fixes for net, they are:

1) Set on HW_DEAD after HW_PENDING is cleared in the flowtable offload
   to ensure GC does not zap it, from Jérémy Jean.
 
2) Hold the nfnetlink_queue mutex while removing the queue instance
   from the netlink notifier that handles NETLINK_URELEASE to fix a
   possible race with the UNBIND command. From Florian Westphal.
 
3) Reject route with NULL rt6i_idev in ip6t_rpfilter. From Weiming Shi.
 
4) Reject rtinfo->addrnr set to zero from ip6t_rt .checkentry path.
   This also fortifies the datapath loop as per Florian's request.
   From Luxiao Xu.
 
5) Fix checksuming in nft_synproxy for IPv6, from Karl Mehltretter.
 
6) Revalidate ihl before calling icmp_send() in IPVS,
   from Julian Anastasov.
 
7) Fix suspicious RCU usage splat in ctnetlink with expectations.
 
8) Check for expired catchall elements in the insert and deactivate
   path. From Aohan Mei.

Please, pull these changes from:

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

Thanks.

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

The following changes since commit 3b95a04eb5f95bf6a016a1bb9ff37d3eee48de63:

  Merge branch 'mptcp-misc-fixes-for-v7-3-rc4' (2026-09-17 08:14:39 -0700)

are available in the Git repository at:

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

for you to fetch changes up to 70194dc37670bd08e44b471389861cc01bd3a3c9:

  netfilter: nf_tables: skip expired catchall elements on insert and delete (2026-09-18 11:24:05 +0200)

----------------------------------------------------------------
netfilter pull request 26-09-18

----------------------------------------------------------------
Aohan Mei (1):
      netfilter: nf_tables: skip expired catchall elements on insert and delete

Florian Westphal (1):
      netfilter: nfnetlink_queue: hold nfnl mutex in event notifier

Julian Anastasov (1):
      ipvs: revalidate ihl before icmp_send

Jérémy Jean (1):
      netfilter: flowtable: publish HW_DEAD after worker is done

Karl Mehltretter (1):
      netfilter: nft_synproxy: use the family-aware checksum helper

Luxiao Xu (1):
      netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read

Naman Gulati (1):
      netfilter: ctnetlink: fix suspicious RCU usage in expect_iter_name

Weiming Shi (1):
      netfilter: ip6t_rpfilter: reject routes without inet6_dev

 net/ipv6/netfilter/ip6t_rpfilter.c    |  2 +-
 net/ipv6/netfilter/ip6t_rt.c          | 11 ++++++++---
 net/netfilter/ipvs/ip_vs_core.c       |  6 ++++++
 net/netfilter/nf_conntrack_netlink.c  |  3 ++-
 net/netfilter/nf_flow_table_offload.c |  7 ++++++-
 net/netfilter/nf_tables_api.c         | 10 ++++++++--
 net/netfilter/nfnetlink_queue.c       |  8 +++++---
 net/netfilter/nft_synproxy.c          |  3 ++-
 8 files changed, 38 insertions(+), 12 deletions(-)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH net 1/8] netfilter: flowtable: publish HW_DEAD after worker is done
  2026-09-18 11:28 [PATCH net,v2 0/8] Netfilter/IPVS fixes for net Pablo Neira Ayuso
@ 2026-09-18 11:28 ` Pablo Neira Ayuso
  2026-09-19 13:34   ` netdev-bot+sashiko
  2026-09-21 22:20   ` patchwork-bot+netdevbpf
  2026-09-18 11:28 ` [PATCH net 2/8] netfilter: nfnetlink_queue: hold nfnl mutex in event notifier Pablo Neira Ayuso
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-18 11:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet, horms, fw, ja

From: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>

flow_offload_work_del() sets NF_FLOW_HW_DEAD before the work handler
clears NF_FLOW_HW_PENDING. Once a flow is both HW_DYING and HW_DEAD, a
concurrent garbage collection pass can remove it and schedule it for RCU
freeing.

The offload worker holds neither an RCU read lock nor a reference to the
flow. If it is preempted after publishing HW_DEAD, the RCU callback can
free the flow before the worker resumes and clears HW_PENDING, resulting
in a use-after-free.

Move HW_DEAD publication to the common worker epilogue after the pending
bit is cleared, making it the final flow access by destroy work. Order all
preceding flow accesses before publishing the bit that allows garbage
collection to free the object.

Fixes: 2c8897953f3b ("netfilter: flowtable: Add pending bit for offload work")
Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_flow_table_offload.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index 801a3dd9ceea..6757fd89c1f1 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -995,7 +995,6 @@ static void flow_offload_work_del(struct flow_offload_work *offload)
 	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_ORIGINAL);
 	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
 		flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_REPLY);
-	set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
 }
 
 static void flow_offload_tuple_stats(struct flow_offload_work *offload,
@@ -1059,6 +1058,12 @@ static void flow_offload_work_handler(struct work_struct *work)
 	}
 
 	clear_bit(NF_FLOW_HW_PENDING, &offload->flow->flags);
+	if (offload->cmd == FLOW_CLS_DESTROY) {
+		/* Publish after the worker's last flow access. */
+		smp_mb__before_atomic();
+		set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
+	}
+
 	kfree(offload);
 }
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net 2/8] netfilter: nfnetlink_queue: hold nfnl mutex in event notifier
  2026-09-18 11:28 [PATCH net,v2 0/8] Netfilter/IPVS fixes for net Pablo Neira Ayuso
  2026-09-18 11:28 ` [PATCH net 1/8] netfilter: flowtable: publish HW_DEAD after worker is done Pablo Neira Ayuso
@ 2026-09-18 11:28 ` Pablo Neira Ayuso
  2026-09-18 11:28 ` [PATCH net 3/8] netfilter: ip6t_rpfilter: reject routes without inet6_dev Pablo Neira Ayuso
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-18 11:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet, horms, fw, ja

From: Florian Westphal <fw@strlen.de>

We must serialize the release notifier and the config netlink function.
A concurrent thread can issue close() which can call the release function
while unrelated socket processes UNBIND request for same portid:

Oops: general protection fault, [..]
RIP: 0010:__instance_destroy+0x60/0x210 [nfnetlink_queue]
Call Trace:
 nfqnl_recv_config+0x9b0/0xdc0 [nfnetlink_queue]
 nfnetlink_rcv_msg+0x7c2/0xeb0
 ? __pfx_nfnetlink_rcv_msg+0x10/0x10

After this, parallel UNBIND and URELEASE events are impossible.

This change isn't nice, but its the shortest fix given instances
are not refcounted and the nfnetlink config callback drops the
rcu read lock early due to need for sleeping allocations.

Fixes: 7af4cc3fa158 ("[NETFILTER]: Add "nfnetlink_queue" netfilter queue handler over nfnetlink")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink_queue.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index c727668b0c5b..a3bc00280051 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -1593,6 +1593,7 @@ nfqnl_rcv_nl_event(struct notifier_block *this,
 	if (event == NETLINK_URELEASE && n->protocol == NETLINK_NETFILTER) {
 		int i;
 
+		nfnl_lock(NFNL_SUBSYS_QUEUE);
 		/* destroy all instances for this portid */
 		spin_lock(&q->instances_lock);
 		for (i = 0; i < INSTANCE_BUCKETS; i++) {
@@ -1606,6 +1607,7 @@ nfqnl_rcv_nl_event(struct notifier_block *this,
 			}
 		}
 		spin_unlock(&q->instances_lock);
+		nfnl_unlock(NFNL_SUBSYS_QUEUE);
 	}
 	return NOTIFY_DONE;
 }
@@ -1925,9 +1927,9 @@ static int nfqnl_recv_config(struct sk_buff *skb, const struct nfnl_info *info,
 
 	/* Lookup queue under RCU. After peer_portid check (or for new queue
 	 * in BIND case), the queue is owned by the socket sending this message.
-	 * A socket cannot simultaneously send a message and close, so while
-	 * processing this CONFIG message, nfqnl_rcv_nl_event() (triggered by
-	 * socket close) cannot destroy this queue. Safe to use without RCU.
+	 * nfqnl_rcv_nl_event() will block on the nfnl subsys mutex that is
+	 * held by the caller, so the queue cannot be destroyed in parallel,
+	 * even after we drop the RCU read lock.
 	 */
 	rcu_read_lock();
 	queue = instance_lookup(q, queue_num);
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net 3/8] netfilter: ip6t_rpfilter: reject routes without inet6_dev
  2026-09-18 11:28 [PATCH net,v2 0/8] Netfilter/IPVS fixes for net Pablo Neira Ayuso
  2026-09-18 11:28 ` [PATCH net 1/8] netfilter: flowtable: publish HW_DEAD after worker is done Pablo Neira Ayuso
  2026-09-18 11:28 ` [PATCH net 2/8] netfilter: nfnetlink_queue: hold nfnl mutex in event notifier Pablo Neira Ayuso
@ 2026-09-18 11:28 ` Pablo Neira Ayuso
  2026-09-18 11:28 ` [PATCH net 4/8] netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read Pablo Neira Ayuso
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-18 11:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet, horms, fw, ja

From: Weiming Shi <bestswngs@gmail.com>

ip6_route_lookup() can return an error-free route whose rt6i_idev is
NULL. Lowering an external nexthop device's MTU below IPV6_MIN_MTU tears
down its inet6_dev while fib6_ifdown() leaves routes using nexthop objects
in the FIB. An unprivileged user can construct this state with rtnetlink
in a private user and network namespace, then trigger a NULL dereference
through an IPv6 rpfilter lookup:

  Oops: general protection fault, probably for non-canonical address
  0xdffffc0000000000
  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
  RIP: rpfilter_mt (net/ipv6/netfilter/ip6t_rpfilter.c:75)
  Call Trace:
  ip6t_do_table (net/ipv6/netfilter/ip6_tables.c:316)
  nf_hook_slow (net/netfilter/core.c:619)
  ipv6_rcv (net/ipv6/ip6_input.c:351)
  __netif_receive_skb_one_core (net/core/dev.c:6216)
  process_backlog (net/core/dev.c:6680)
  __napi_poll (net/core/dev.c:7739)
  net_rx_action (net/core/dev.c:7959)
  handle_softirqs (kernel/softirq.c:622)
  do_softirq.part.0 (kernel/softirq.c:523)
  __local_bh_enable_ip (kernel/softirq.c:450)
  __dev_queue_xmit (net/core/dev.c:4913)
  packet_sendmsg (net/packet/af_packet.c:3139)
  __sys_sendto (net/socket.c:2252)
  __x64_sys_sendto (net/socket.c:2259)
  do_syscall_64 (arch/x86/entry/syscall_64.c:94)
  entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
  Kernel panic - not syncing: Fatal exception in interrupt

Reject routes without an inet6_dev immediately after lookup. Such routes
are not eligible for reverse-path filtering, and the check protects all
later rt6i_idev dereferences.

Fixes: e26f9a480fb6 ("netfilter: add ipv6 reverse path filter match")
Reported-by: co+459f67f4d8af8ce6@bugs.sh
Closes: https://lore.kernel.org/all/VtWUkE8QzJt5CroTj2V2v3ZQ0gwbXZ7nq7I3@bugs.sh/
Suggested-by: Florian Westphal <fw@strlen.de>
Assisted-by: Claude:gpt-5
Cc: stable@vger.kernel.org
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv6/netfilter/ip6t_rpfilter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
index 67c87a88cde4..b5def30c3127 100644
--- a/net/ipv6/netfilter/ip6t_rpfilter.c
+++ b/net/ipv6/netfilter/ip6t_rpfilter.c
@@ -61,7 +61,7 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
 		fl6.flowi6_oif = dev->ifindex;
 
 	rt = (void *)ip6_route_lookup(net, &fl6, skb, lookup_flags);
-	if (rt->dst.error)
+	if (rt->dst.error || !rt->rt6i_idev)
 		goto out;
 
 	if (rt->rt6i_flags & (RTF_REJECT|RTF_ANYCAST))
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net 4/8] netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read
  2026-09-18 11:28 [PATCH net,v2 0/8] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (2 preceding siblings ...)
  2026-09-18 11:28 ` [PATCH net 3/8] netfilter: ip6t_rpfilter: reject routes without inet6_dev Pablo Neira Ayuso
@ 2026-09-18 11:28 ` Pablo Neira Ayuso
  2026-09-19 13:34   ` netdev-bot+sashiko
  2026-09-18 11:28 ` [PATCH net 5/8] netfilter: nft_synproxy: use the family-aware checksum helper Pablo Neira Ayuso
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-18 11:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet, horms, fw, ja

From: Luxiao Xu <rakukuip@gmail.com>

rt_mt6_check() permits rules to be configured with rtinfo->addrnr == 0
even when address matching (IP6T_RT_FST_MASK) is requested.

In the IP6T_RT_FST_NSTRICT path, rt_mt6() evaluates packet routing
addresses against rtinfo->addrs[i] and terminates backwards at the bottom
of the loop:

    if (ipv6_addr_equal(ap, &rtinfo->addrs[i])) {
        i++;
    }
    if (i == rtinfo->addrnr)
        break;

When addrnr is 0, if the first packet address matches rtinfo->addrs[0],
i is incremented to 1. Because i is now strictly greater than addrnr (0),
the loop termination condition (i == rtinfo->addrnr) is bypassed and will
never be satisfied.

If a crafted IPv6 packet contains matching routing addresses, i will
advance past IP6T_RT_HOPS (16). The subsequent call to ipv6_addr_equal()
reads beyond struct ip6t_rt, triggering UBSAN/KASAN out-of-bounds warnings
or kernel panics.

Fix this by:
1. Rejecting rules in rt_mt6_check() where IP6T_RT_FST_MASK is set but
   rtinfo->addrnr is zero.
2. In rt_mt6(), moving the termination condition (i < rtinfo->addrnr)
   into the for-loop header condition and removing the backwards break
   at the end of the loop body.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Suggested-by: Florian Westphal <fw@strlen.de>
Assisted-by: LLM
Signed-off-by: Luxiao Xu <rakukuip@gmail.com>
Signed-off-by: Ren Wei <weir@nebusec.ai>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv6/netfilter/ip6t_rt.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/netfilter/ip6t_rt.c b/net/ipv6/netfilter/ip6t_rt.c
index 8051425213dd..9880faf3cc7d 100644
--- a/net/ipv6/netfilter/ip6t_rt.c
+++ b/net/ipv6/netfilter/ip6t_rt.c
@@ -96,7 +96,8 @@ static bool rt_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 			unsigned int i = 0;
 
 			for (temp = 0;
-			     temp < (unsigned int)((hdrlen - 8) / 16);
+			     temp < (unsigned int)((hdrlen - 8) / 16) &&
+			     i < rtinfo->addrnr;
 			     temp++) {
 				ap = skb_header_pointer(skb,
 							ptr
@@ -112,8 +113,6 @@ static bool rt_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 
 				if (ipv6_addr_equal(ap, &rtinfo->addrs[i]))
 					i++;
-				if (i == rtinfo->addrnr)
-					break;
 			}
 			if (i == rtinfo->addrnr)
 				return ret;
@@ -162,6 +161,12 @@ static int rt_mt6_check(const struct xt_mtchk_param *par)
 		pr_info_ratelimited("too many addresses specified\n");
 		return -EINVAL;
 	}
+
+	if ((rtinfo->flags & IP6T_RT_FST_MASK) && !rtinfo->addrnr) {
+		pr_info_ratelimited("address list match requested but addrnr is 0\n");
+		return -EINVAL;
+	}
+
 	if ((rtinfo->flags & (IP6T_RT_RES | IP6T_RT_FST_MASK)) &&
 	    (!(rtinfo->flags & IP6T_RT_TYP) ||
 	     (rtinfo->rt_type != 0) ||
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net 5/8] netfilter: nft_synproxy: use the family-aware checksum helper
  2026-09-18 11:28 [PATCH net,v2 0/8] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (3 preceding siblings ...)
  2026-09-18 11:28 ` [PATCH net 4/8] netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read Pablo Neira Ayuso
@ 2026-09-18 11:28 ` Pablo Neira Ayuso
  2026-09-18 11:28 ` [PATCH net 6/8] ipvs: revalidate ihl before icmp_send Pablo Neira Ayuso
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-18 11:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet, horms, fw, ja

From: Karl Mehltretter <kmehltretter@gmail.com>

nft_synproxy_do_eval() verifies the TCP checksum before it switches on
skb->protocol.  It uses nf_ip_checksum(), which constructs an IPv4
pseudo header and relies on the IPv4 header checksum when folding the
whole skb.  Neither operation is valid for an IPv6 packet.

A correctly checksummed IPv6 segment can therefore fail verification
when it reaches the hook as CHECKSUM_NONE or, at NF_INET_LOCAL_IN,
CHECKSUM_COMPLETE.  nft_synproxy_do_eval() returns NF_DROP before
nft_synproxy_eval_v6() can send a SYN-ACK.

nft_synproxy_validate() deliberately admits NFPROTO_IPV6 and
NFPROTO_INET, and the xtables counterpart ip6t_SYNPROXY.c already calls
nf_ip6_checksum().

Use nf_checksum() with nft_pf() so the checksum helper dispatches to the
packet family's implementation.

Fixes: ad49d86e07a4 ("netfilter: nf_tables: Add synproxy support")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_synproxy.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nft_synproxy.c b/net/netfilter/nft_synproxy.c
index 9ed288c9d168..554a96a000f4 100644
--- a/net/netfilter/nft_synproxy.c
+++ b/net/netfilter/nft_synproxy.c
@@ -118,7 +118,8 @@ static void nft_synproxy_do_eval(const struct nft_synproxy *priv,
 		return;
 	}
 
-	if (nf_ip_checksum(skb, nft_hook(pkt), thoff, IPPROTO_TCP)) {
+	if (nf_checksum(skb, nft_hook(pkt), thoff, IPPROTO_TCP,
+			nft_pf(pkt))) {
 		regs->verdict.code = NF_DROP;
 		return;
 	}
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net 6/8] ipvs: revalidate ihl before icmp_send
  2026-09-18 11:28 [PATCH net,v2 0/8] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (4 preceding siblings ...)
  2026-09-18 11:28 ` [PATCH net 5/8] netfilter: nft_synproxy: use the family-aware checksum helper Pablo Neira Ayuso
@ 2026-09-18 11:28 ` Pablo Neira Ayuso
  2026-09-18 11:28 ` [PATCH net 7/8] netfilter: ctnetlink: fix suspicious RCU usage in expect_iter_name Pablo Neira Ayuso
  2026-09-18 11:28 ` [PATCH net 8/8] netfilter: nf_tables: skip expired catchall elements on insert and delete Pablo Neira Ayuso
  7 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-18 11:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet, horms, fw, ja

From: Julian Anastasov <ja@ssi.bg>

While the outer IP header is already pulled into the skb head, we must
be careful and revalidate the embedded headers after reading them from
the skb frags to prevent possible out-of-bounds access.

One such place reported by Sashiko is ip_vs_in_icmp() where local
process can change the ihl field and after pskb_may_pull() we can see
larger value. Even if icmp_send() has checks to prevent out-of-bounds
access, play safe and add check to drop the packet if the ihl field is
changed.  As the outer headers are pulled, make sure the transport
header is updated too, it was used before commit 7fcc2fe39fed ("net:
icmp: avoid invalid transport header access in icmp_send tracepoint")

Fixes: f2edb9f7706d ("ipvs: implement passive PMTUD for IPIP packets")
Link: https://sashiko.dev/#/patchset/20260806105211.34622-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_core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index ba0957798bad..fd503f0efb57 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1960,6 +1960,12 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
 		/* Ensure the IP header is present in headroom */
 		if (!pskb_may_pull(skb, hlen_orig))
 			goto ignore_tunnel;
+		skb_set_transport_header(skb, hlen_orig);
+		/* Before now we may used ihl from skb frag, revalidate it after
+		 * copying it into skb head to prevent out-of-bounds access
+		 */
+		if (ip_hdr(skb)->ihl * 4 != hlen_orig)
+			goto ignore_tunnel;
 		IP_VS_DBG(12, "Sending ICMP for %pI4->%pI4: t=%u, c=%u, i=%u\n",
 			&ip_hdr(skb)->saddr, &ip_hdr(skb)->daddr,
 			type, code, ntohl(info));
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net 7/8] netfilter: ctnetlink: fix suspicious RCU usage in expect_iter_name
  2026-09-18 11:28 [PATCH net,v2 0/8] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (5 preceding siblings ...)
  2026-09-18 11:28 ` [PATCH net 6/8] ipvs: revalidate ihl before icmp_send Pablo Neira Ayuso
@ 2026-09-18 11:28 ` Pablo Neira Ayuso
  2026-09-18 11:28 ` [PATCH net 8/8] netfilter: nf_tables: skip expired catchall elements on insert and delete Pablo Neira Ayuso
  7 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-18 11:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet, horms, fw, ja

From: Naman Gulati <namangulati@google.com>

expect_iter_name() is invoked by nf_ct_expect_iterate_net() under
spin_lock_bh(&nf_conntrack_expect_lock). It does not hold
rcu_read_lock().

When accessing exp->helper with rcu_dereference() in syzbot's report,
lockdep warns:

  =============================
  WARNING: suspicious RCU usage
  syzkaller #0 Not tainted
  -----------------------------
  net/netfilter/nf_conntrack_netlink.c:3393 suspicious rcu_dereference_check() usage!

  locks held by syz-executor381/5628: 2, last CPU#1:
   #0: ffffffff9aee42a0 (nfnl_subsys_ctnetlink_exp){+.+.}-{4:4},
       at: nfnetlink_rcv_msg+0xa69/0x12b0
   #1: ffffffff8ea74d58 (nf_conntrack_expect_lock){+...}-{3:3},
       at: nf_ct_expect_iterate_net+0x38/0x180

  Call Trace:
   <TASK>
   dump_stack_lvl+0xe8/0x150
   lockdep_rcu_suspicious+0x140/0x1d0
   expect_iter_name+0xfb/0x100
   nf_ct_expect_iterate_net+0xf2/0x180
   ctnetlink_del_expect+0x45d/0x640
   nfnetlink_rcv_msg+0xcc2/0x12b0
   netlink_rcv_skb+0x226/0x4a0
   nfnetlink_rcv+0x2b9/0x28c0
   netlink_unicast+0x7bd/0x940
   netlink_sendmsg+0x813/0xb40
   ____sys_sendmsg+0x54e/0x850
   ___sys_sendmsg+0x2a5/0x360
   __sys_sendmsg+0x2a5/0x360
   do_syscall_64+0x166/0x520
   entry_SYSCALL_64_after_hwframe+0x77/0x7f

Use rcu_dereference_protected() with lockdep_is_held() on
nf_conntrack_expect_lock instead, similar to expect_iter_me() in
nf_conntrack_helper.c.

Fixes: f01794106042 ("netfilter: nf_conntrack_expect: use expect->helper")
Reported-by: syzbot+4bd730aede2791e40bdf@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6aa4a377.f81106d8.2ab401.0024.GAE@google.com/T/#u
Signed-off-by: Naman Gulati <namangulati@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_netlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 579ada063b1b..4e5d7c701436 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -3392,7 +3392,8 @@ static bool expect_iter_name(struct nf_conntrack_expect *exp, void *data)
 	struct nf_conntrack_helper *helper;
 	const char *name = data;
 
-	helper = rcu_dereference(exp->helper);
+	helper = rcu_dereference_protected(exp->helper,
+					   lockdep_is_held(&nf_conntrack_expect_lock));
 	if (!helper)
 		return false;
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net 8/8] netfilter: nf_tables: skip expired catchall elements on insert and delete
  2026-09-18 11:28 [PATCH net,v2 0/8] Netfilter/IPVS fixes for net Pablo Neira Ayuso
                   ` (6 preceding siblings ...)
  2026-09-18 11:28 ` [PATCH net 7/8] netfilter: ctnetlink: fix suspicious RCU usage in expect_iter_name Pablo Neira Ayuso
@ 2026-09-18 11:28 ` Pablo Neira Ayuso
  2026-09-19 13:34   ` netdev-bot+sashiko
  7 siblings, 1 reply; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-18 11:28 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet, horms, fw, ja

From: Aohan Mei <henrymei@tencent.com>

nft_setelem_catchall_insert() looks up duplicates with
nft_set_elem_active() only, while nft_set_catchall_lookup() and the
dump path additionally skip expired elements.

Once a catchall element with a timeout expires, this predicate drift
makes it invisible to userspace dumps, yet it still blocks
re-insertion: with NLM_F_EXCL the request fails with -EEXIST, and
without it the request reports success but silently inserts nothing.
The stale entry only goes away when the (user-tunable) gc interval
elapses, so the catchall rule may silently stop matching for an
arbitrarily long time after its first expiration.

The delete path shows the same drift: nft_setelem_catchall_deactivate()
picks the first active-next entry in the catchall list, so with an
expired entry still pending GC it retires the stale entry instead of
the fresh one, and it deactivates an element that userspace no longer
sees instead of failing with -ENOENT.

Align both walks with the lookup and dump predicates: only an element
that is active and not expired counts as a duplicate or delete
candidate, using the per-netns timestamp taken at transaction start,
in line with the set backend .insert/.deactivate and catchall GC sync
paths.

Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Fixes: aaa31047a6d2 ("netfilter: nftables: add catch-all set element support")
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c0b754a2d45b..b59628e6240c 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -6995,11 +6995,14 @@ static int nft_setelem_catchall_insert(const struct net *net,
 {
 	struct nft_set_elem_catchall *catchall;
 	u8 genmask = nft_genmask_next(net);
+	u64 tstamp = nft_net_tstamp(net);
 	struct nft_set_ext *ext;
 
 	list_for_each_entry(catchall, &set->catchall_list, list) {
 		ext = nft_set_elem_ext(set, catchall->elem);
-		if (nft_set_elem_active(ext, genmask)) {
+		if (nft_set_elem_active(ext, genmask) &&
+		    !__nft_set_elem_expired(ext, tstamp) &&
+		    !nft_set_elem_is_dead(ext)) {
 			*priv = catchall->elem;
 			return -EEXIST;
 		}
@@ -7092,11 +7095,14 @@ static int nft_setelem_catchall_deactivate(const struct net *net,
 					   struct nft_set_elem *elem)
 {
 	struct nft_set_elem_catchall *catchall;
+	u64 tstamp = nft_net_tstamp(net);
 	struct nft_set_ext *ext;
 
 	list_for_each_entry(catchall, &set->catchall_list, list) {
 		ext = nft_set_elem_ext(set, catchall->elem);
-		if (!nft_is_active_next(net, ext))
+		if (!nft_is_active_next(net, ext) ||
+		    __nft_set_elem_expired(ext, tstamp) ||
+		    nft_set_elem_is_dead(ext))
 			continue;
 
 		kfree(elem->priv);
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH net 1/8] netfilter: flowtable: publish HW_DEAD after worker is done
  2026-09-18 11:28 ` [PATCH net 1/8] netfilter: flowtable: publish HW_DEAD after worker is done Pablo Neira Ayuso
@ 2026-09-19 13:34   ` netdev-bot+sashiko
  2026-09-19 14:47     ` Pablo Neira Ayuso
  2026-09-21 22:20   ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-09-19 13:34 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, davem, netdev, kuba, pabeni, edumazet, horms, fw,
	ja

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

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

- [High] Incomplete fix: the FLOW_CLS_STATS (and, in a narrower window,
  FLOW_CLS_REPLACE) offload work items still hold an unreferenced, non-…

--

netfilter: flowtable: publish HW_DEAD after worker is done

The NF_FLOW_HW_DEAD publication is moved out of flow_offload_work_del()
into the common flow_offload_work_handler() epilogue, after
NF_FLOW_HW_PENDING is cleared, so that the destroy work item does not
touch the flow after garbage collection is allowed to free it.

> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index 801a3dd9ceea..6757fd89c1f1 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -995,7 +995,6 @@ static void flow_offload_work_del(struct flow_offload_work *offload)
>  	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_ORIGINAL);
>  	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
>  		flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_REPLY);
> -	set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
>  }
>  
>  static void flow_offload_tuple_stats(struct flow_offload_work *offload,
> @@ -1059,6 +1058,12 @@ static void flow_offload_work_handler(struct work_struct *work)
>  	}
>  
>  	clear_bit(NF_FLOW_HW_PENDING, &offload->flow->flags);
> +	if (offload->cmd == FLOW_CLS_DESTROY) {
> +		/* Publish after the worker's last flow access. */
> +		smp_mb__before_atomic();
> +		set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
> +	}
> +
>  	kfree(offload);
>  }

[Severity: High]
Does this reordering also cover the FLOW_CLS_STATS and FLOW_CLS_REPLACE work
items?  The clear_bit() above runs for every command, and for stats work the
flow can already be gone by the time the worker reaches it.

The sequence I think is still possible, all on a flow that has NF_FLOW_HW set:

A remote FIN/RST marks the flow closing in nf_flow_state_check():

net/netfilter/nf_flow_table_ip.c:nf_flow_state_check() {
	if ((tcph->fin || tcph->rst) &&
	    !test_bit(NF_FLOW_CLOSING, &flow->flags))
		set_bit(NF_FLOW_CLOSING, &flow->flags);
}

nf_flow_offload_gc_step() then queues the destroy work, and the flow stays
linked in the rhashtable:

net/netfilter/nf_flow_table_core.c:nf_flow_offload_gc_step() {
	} else if (test_bit(NF_FLOW_CLOSING, &flow->flags) &&
		   test_bit(NF_FLOW_HW, &flow->flags) &&
		   !test_bit(NF_FLOW_HW_DYING, &flow->flags)) {
		nf_flow_offload_del(flow_table, flow);
	} else if (test_bit(NF_FLOW_HW, &flow->flags)) {
		nf_flow_offload_stats(flow_table, flow);
	}
}

The destroy worker now clears NF_FLOW_HW_PENDING and publishes
NF_FLOW_HW_DEAD.  Since NF_FLOW_HW, NF_FLOW_HW_DYING and NF_FLOW_HW_DEAD are
never cleared again anywhere in the tree, a later garbage collection pass on
the same flow falls through to the last branch above and queues a stats work
item, because nf_flow_offload_work_alloc() only gates on the pending bit:

net/netfilter/nf_flow_table_offload.c:nf_flow_offload_work_alloc() {
	if (test_and_set_bit(NF_FLOW_HW_PENDING, &flow->flags))
		return NULL;
	...
	offload->flow = flow;
}

So a stats work item can exist for a flow that already has HW_DYING and
HW_DEAD set.  When NF_FLOW_TEARDOWN is then set (expiry, ct dying, dst
invalid, netdev down, or a SYN on the closing flow via
nf_flow_state_check() -> flow_offload_teardown()), the next pass frees it:

net/netfilter/nf_flow_table_core.c:nf_flow_offload_gc_step() {
	if (teardown) {
		if (test_bit(NF_FLOW_HW, &flow->flags)) {
			if (!test_bit(NF_FLOW_HW_DYING, &flow->flags))
				nf_flow_offload_del(flow_table, flow);
			else if (test_bit(NF_FLOW_HW_DEAD, &flow->flags))
				flow_offload_del(flow_table, flow);
	...
}

flow_offload_del() -> flow_offload_free() -> call_rcu(&flow->rcu_head,
flow_offload_free_rcu) waits only for RCU readers.  The stats worker holds no
reference and no RCU read lock, and it sleeps in
down_read(&flowtable->flow_block_lock) and in the driver callbacks, so can
the grace period elapse while flow_offload_work_stats() is still using the
flow here?

net/netfilter/nf_flow_table_offload.c:flow_offload_work_stats() {
	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
	...
	offload->flow->timeout = max_t(u64, offload->flow->timeout,
				       lastused + flow_offload_get_timeout(offload->flow));
}

and then in the shared epilogue:

	clear_bit(NF_FLOW_HW_PENDING, &offload->flow->flags);

That looks like the same "worker writes flow flags after the flow may have
been freed" pattern the commit message describes, just reached through the
stats work item instead of the destroy one.

The ordering in nf_flow_table_gc_cleanup() seems to make this explicit on the
netdev-down and flowtable-delete paths:

net/netfilter/nf_flow_table_core.c:nf_flow_table_gc_cleanup() {
	nf_flow_table_iterate(flowtable, nf_flow_table_do_cleanup, dev);
	flush_delayed_work(&flowtable->gc_work);
	nf_flow_table_offload_flush(flowtable);
}

Teardown is set, the gc flush frees the HW_DEAD flows, and only afterwards
are the add/del/stats workqueues drained.  Should the flush of the offload
workqueues happen before the gc flush here?

Would it make sense for the free decision in nf_flow_offload_gc_step() to
also consult NF_FLOW_HW_PENDING, or for the work items to take a reference
on the flow (or for work admission to be refused once NF_FLOW_HW_DYING is
set), rather than only reordering the two bit operations in the destroy
worker?

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net 4/8] netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read
  2026-09-18 11:28 ` [PATCH net 4/8] netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read Pablo Neira Ayuso
@ 2026-09-19 13:34   ` netdev-bot+sashiko
  2026-09-19 15:02     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-09-19 13:34 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, davem, netdev, kuba, pabeni, edumazet, horms, fw,
	ja

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

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

- [Low] Validator/matcher flag-semantics disagreement in
  net/ipv6/netfilter/ip6t_rt.c.

--

netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read

This adds a loop-header bound (i < rtinfo->addrnr) to the non-strict
address matching loop in rt_mt6(), removing the backwards break at the
bottom of the loop.  It also adds a rt_mt6_check() rejection for rules
that set IP6T_RT_FST_MASK with rtinfo->addrnr == 0.

> diff --git a/net/ipv6/netfilter/ip6t_rt.c b/net/ipv6/netfilter/ip6t_rt.c
> index 8051425213dd..9880faf3cc7d 100644
> --- a/net/ipv6/netfilter/ip6t_rt.c
> +++ b/net/ipv6/netfilter/ip6t_rt.c
> @@ -96,7 +96,8 @@ static bool rt_mt6(const struct sk_buff *skb, struct xt_action_param *par)
>  			unsigned int i = 0;
>  
>  			for (temp = 0;
> -			     temp < (unsigned int)((hdrlen - 8) / 16);
> +			     temp < (unsigned int)((hdrlen - 8) / 16) &&
> +			     i < rtinfo->addrnr;
>  			     temp++) {
>  				ap = skb_header_pointer(skb,
>  							ptr
> @@ -112,8 +113,6 @@ static bool rt_mt6(const struct sk_buff *skb, struct xt_action_param *par)
>  
>  				if (ipv6_addr_equal(ap, &rtinfo->addrs[i]))
>  					i++;
> -				if (i == rtinfo->addrnr)
> -					break;
>  			}
>  			if (i == rtinfo->addrnr)
>  				return ret;

[ ... ]

> @@ -162,6 +161,12 @@ static int rt_mt6_check(const struct xt_mtchk_param *par)
>  		pr_info_ratelimited("too many addresses specified\n");
>  		return -EINVAL;
>  	}
> +
> +	if ((rtinfo->flags & IP6T_RT_FST_MASK) && !rtinfo->addrnr) {
> +		pr_info_ratelimited("address list match requested but addrnr is 0\n");
> +		return -EINVAL;
> +	}
> +

[Severity: Low]
Is the mask used here wider than the flag the matcher actually keys on?
IP6T_RT_FST_MASK is 0x30, covering both bits:

include/uapi/linux/netfilter_ipv6/ip6t_rt.h:
    #define IP6T_RT_FST_MASK	0x30
    #define IP6T_RT_FST 		0x10
    #define IP6T_RT_FST_NSTRICT	0x20

But rt_mt6() gates every access to rtinfo->addrs[] and rtinfo->addrnr on
IP6T_RT_FST alone:

net/ipv6/netfilter/ip6t_rt.c:rt_mt6() {
	if (!(rtinfo->flags & IP6T_RT_FST)) {
		return ret;
	} else if (rtinfo->flags & IP6T_RT_FST_NSTRICT) {
	...
}

So a rule with only IP6T_RT_FST_NSTRICT set and addrnr == 0 never touches
addrs[] at match time, yet it is now rejected with -EINVAL at load time.

Would narrowing the new test to IP6T_RT_FST avoid that load-time behaviour
change?  Such blobs can only come from direct IP6T_SO_SET_REPLACE or
nft_compat callers, since libip6t_rt requires --rt-0-addrs before
--rt-0-not-strict, so no ruleset produced by iptables appears to be
affected.

Does the new check also make the strict branch's addrnr == 0 case
unreachable?  That branch has a meaningful zero-address semantic, matching
a routing header that carries no addresses at all (hdrlen == 8):

net/ipv6/netfilter/ip6t_rt.c:rt_mt6() {
	...
			if (temp == rtinfo->addrnr &&
			    temp == (unsigned int)((hdrlen - 8) / 16))
				return ret;
	...
}

With addrnr == 0 now refused at load time, that comparison can no longer be
satisfied with addrnr == 0.

One more thing worth noting: is the new checkentry test needed for memory
safety at all?  The loop-header change bounds i to addrnr, and addrnr is
already capped at IP6T_RT_HOPS by the preceding test, so &rtinfo->addrs[i]
stays inside the 16-entry array on its own.

Separately, and not something this patch introduces: a rule with
IP6T_RT_TYP | IP6T_RT_FST_NSTRICT, rt_type 0 and addrnr > 0 still passes
rt_mt6_check() as an address list match, but rt_mt6() returns at the
!(rtinfo->flags & IP6T_RT_FST) test and silently ignores every configured
address.  Should the validator and the matcher agree on which flag means
"match the address list"?

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net 8/8] netfilter: nf_tables: skip expired catchall elements on insert and delete
  2026-09-18 11:28 ` [PATCH net 8/8] netfilter: nf_tables: skip expired catchall elements on insert and delete Pablo Neira Ayuso
@ 2026-09-19 13:34   ` netdev-bot+sashiko
  2026-09-19 14:57     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 16+ messages in thread
From: netdev-bot+sashiko @ 2026-09-19 13:34 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, davem, netdev, kuba, pabeni, edumazet, horms, fw,
	ja

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

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

- [Medium] The relaxed duplicate predicate in
  nft_setelem_catchall_insert() (nf_tables_api.c:7003-7016) removes the…

--

netfilter: nf_tables: skip expired catchall elements on insert and delete

This aligns nft_setelem_catchall_insert() and
nft_setelem_catchall_deactivate() with the lookup and dump predicates, so
an expired catchall element no longer counts as a duplicate and no longer
gets picked as the delete candidate. Both walks now also test
__nft_set_elem_expired() with the per-netns transaction timestamp and
nft_set_elem_is_dead().

> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index c0b754a2d45b..b59628e6240c 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -6995,11 +6995,14 @@ static int nft_setelem_catchall_insert(const struct net *net,
>  {
>  	struct nft_set_elem_catchall *catchall;
>  	u8 genmask = nft_genmask_next(net);
> +	u64 tstamp = nft_net_tstamp(net);
>  	struct nft_set_ext *ext;
>  
>  	list_for_each_entry(catchall, &set->catchall_list, list) {
>  		ext = nft_set_elem_ext(set, catchall->elem);
> -		if (nft_set_elem_active(ext, genmask)) {
> +		if (nft_set_elem_active(ext, genmask) &&
> +		    !__nft_set_elem_expired(ext, tstamp) &&
> +		    !nft_set_elem_is_dead(ext)) {
>  			*priv = catchall->elem;
>  			return -EEXIST;
>  		}
>  	}

[Severity: Medium]
Can repeated expire-then-readd cycles grow set->catchall_list without
bound here?

With the relaxed predicate, the expired entry is skipped but stays linked
and genmask-active until GC runs, so the tail of
nft_setelem_catchall_insert() appends a second entry alongside it:

	catchall = kmalloc_obj(*catchall, GFP_KERNEL_ACCOUNT);
	if (!catchall)
		return -ENOMEM;

	catchall->elem = elem->priv;
	list_add_tail_rcu(&catchall->list, &set->catchall_list);

Unlike ordinary elements, catchall elements are excluded from the size
accounting in nft_add_set_elem():

	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
		unsigned int max = nft_set_maxsize(set), nelems;

		nelems = atomic_inc_return(&set->nelems);
		if (nelems > max)
			set_full = true;
	}

so the "return set_full ? -ENFILE : 0;" limiter that caps accumulation of
ordinary elements does not apply to this path. Is there anything else that
bounds the number of stale catchall entries a set can hold?

Reclamation looks like it depends entirely on GC, and the interval is
user-tunable with no upper bound (NFTA_SET_GC_INTERVAL is accepted as a u32
in milliseconds and nft_set_gc_interval() just converts it):

include/net/netfilter/nf_tables.h:nft_set_gc_interval() {
	u32 gc_int = READ_ONCE(set->gc_int);

	return gc_int ? msecs_to_jiffies(gc_int) : HZ;
}

For nft_set_rhash and nft_set_rbtree the catchall reclaim runs only from
the delayed gc work requeued with that value, so with a large gc-interval
the window for accumulation is correspondingly large.

There is also a packet-path effect: nft_set_catchall_lookup() walks the
whole list, and new elements are appended at the tail, so every accumulated
stale entry is dereferenced before the live one is found:

	list_for_each_entry_rcu(catchall, &set->catchall_list, list) {
		ext = nft_set_elem_ext(set, catchall->elem);
		if (nft_set_elem_active(ext, genmask) &&
		    !nft_set_elem_expired(ext) &&
		    !nft_set_elem_is_dead(ext))
			return ext;
	}

Would it be preferable to reuse or force-reclaim the stale catchall element
on insert, rather than only ignoring it?

> @@ -7092,11 +7095,14 @@ static int nft_setelem_catchall_deactivate(const struct net *net,
>  					   struct nft_set_elem *elem)
>  {
>  	struct nft_set_elem_catchall *catchall;
> +	u64 tstamp = nft_net_tstamp(net);
>  	struct nft_set_ext *ext;
>  
>  	list_for_each_entry(catchall, &set->catchall_list, list) {
>  		ext = nft_set_elem_ext(set, catchall->elem);
> -		if (!nft_is_active_next(net, ext))
> +		if (!nft_is_active_next(net, ext) ||
> +		    __nft_set_elem_expired(ext, tstamp) ||
> +		    nft_set_elem_is_dead(ext))
>  			continue;
>  
>  		kfree(elem->priv);

[Severity: Medium]
Related to the above: with stale entries now skipped and -ENOENT returned,
per-element DELSETELEM can no longer retire the accumulated entries, so a
full set flush or set deletion appears to be the only way to drop them
before GC fires. Is that the intended behaviour?

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net 1/8] netfilter: flowtable: publish HW_DEAD after worker is done
  2026-09-19 13:34   ` netdev-bot+sashiko
@ 2026-09-19 14:47     ` Pablo Neira Ayuso
  0 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-19 14:47 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: netfilter-devel, davem, netdev, kuba, pabeni, edumazet, horms, fw,
	ja

On Sat, Sep 19, 2026 at 01:34:48PM +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: 1 · Medium: 0 · Low: 0
> 
> - [High] Incomplete fix: the FLOW_CLS_STATS (and, in a narrower window,
>   FLOW_CLS_REPLACE) offload work items still hold an unreferenced, non-…
> 
> --
> 
> netfilter: flowtable: publish HW_DEAD after worker is done
> 
> The NF_FLOW_HW_DEAD publication is moved out of flow_offload_work_del()
> into the common flow_offload_work_handler() epilogue, after
> NF_FLOW_HW_PENDING is cleared, so that the destroy work item does not
> touch the flow after garbage collection is allowed to free it.

This is a pre-existing issue. I have a patch for this already in the
queue for the stats case.

This patch is already improving the situation.

> > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> > index 801a3dd9ceea..6757fd89c1f1 100644
> > --- a/net/netfilter/nf_flow_table_offload.c
> > +++ b/net/netfilter/nf_flow_table_offload.c
> > @@ -995,7 +995,6 @@ static void flow_offload_work_del(struct flow_offload_work *offload)
> >  	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_ORIGINAL);
> >  	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
> >  		flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_REPLY);
> > -	set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
> >  }
> >  
> >  static void flow_offload_tuple_stats(struct flow_offload_work *offload,
> > @@ -1059,6 +1058,12 @@ static void flow_offload_work_handler(struct work_struct *work)
> >  	}
> >  
> >  	clear_bit(NF_FLOW_HW_PENDING, &offload->flow->flags);
> > +	if (offload->cmd == FLOW_CLS_DESTROY) {
> > +		/* Publish after the worker's last flow access. */
> > +		smp_mb__before_atomic();
> > +		set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
> > +	}
> > +
> >  	kfree(offload);
> >  }
> 
> [Severity: High]
> Does this reordering also cover the FLOW_CLS_STATS and FLOW_CLS_REPLACE work
> items?  The clear_bit() above runs for every command, and for stats work the
> flow can already be gone by the time the worker reaches it.
> 
> The sequence I think is still possible, all on a flow that has NF_FLOW_HW set:
> 
> A remote FIN/RST marks the flow closing in nf_flow_state_check():
> 
> net/netfilter/nf_flow_table_ip.c:nf_flow_state_check() {
> 	if ((tcph->fin || tcph->rst) &&
> 	    !test_bit(NF_FLOW_CLOSING, &flow->flags))
> 		set_bit(NF_FLOW_CLOSING, &flow->flags);
> }
> 
> nf_flow_offload_gc_step() then queues the destroy work, and the flow stays
> linked in the rhashtable:
> 
> net/netfilter/nf_flow_table_core.c:nf_flow_offload_gc_step() {
> 	} else if (test_bit(NF_FLOW_CLOSING, &flow->flags) &&
> 		   test_bit(NF_FLOW_HW, &flow->flags) &&
> 		   !test_bit(NF_FLOW_HW_DYING, &flow->flags)) {
> 		nf_flow_offload_del(flow_table, flow);
> 	} else if (test_bit(NF_FLOW_HW, &flow->flags)) {
> 		nf_flow_offload_stats(flow_table, flow);
> 	}
> }
> 
> The destroy worker now clears NF_FLOW_HW_PENDING and publishes
> NF_FLOW_HW_DEAD.  Since NF_FLOW_HW, NF_FLOW_HW_DYING and NF_FLOW_HW_DEAD are
> never cleared again anywhere in the tree, a later garbage collection pass on
> the same flow falls through to the last branch above and queues a stats work
> item, because nf_flow_offload_work_alloc() only gates on the pending bit:
> 
> net/netfilter/nf_flow_table_offload.c:nf_flow_offload_work_alloc() {
> 	if (test_and_set_bit(NF_FLOW_HW_PENDING, &flow->flags))
> 		return NULL;
> 	...
> 	offload->flow = flow;
> }
> 
> So a stats work item can exist for a flow that already has HW_DYING and
> HW_DEAD set.  When NF_FLOW_TEARDOWN is then set (expiry, ct dying, dst
> invalid, netdev down, or a SYN on the closing flow via
> nf_flow_state_check() -> flow_offload_teardown()), the next pass frees it:
> 
> net/netfilter/nf_flow_table_core.c:nf_flow_offload_gc_step() {
> 	if (teardown) {
> 		if (test_bit(NF_FLOW_HW, &flow->flags)) {
> 			if (!test_bit(NF_FLOW_HW_DYING, &flow->flags))
> 				nf_flow_offload_del(flow_table, flow);
> 			else if (test_bit(NF_FLOW_HW_DEAD, &flow->flags))
> 				flow_offload_del(flow_table, flow);
> 	...
> }
> 
> flow_offload_del() -> flow_offload_free() -> call_rcu(&flow->rcu_head,
> flow_offload_free_rcu) waits only for RCU readers.  The stats worker holds no
> reference and no RCU read lock, and it sleeps in
> down_read(&flowtable->flow_block_lock) and in the driver callbacks, so can
> the grace period elapse while flow_offload_work_stats() is still using the
> flow here?
> 
> net/netfilter/nf_flow_table_offload.c:flow_offload_work_stats() {
> 	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
> 	...
> 	offload->flow->timeout = max_t(u64, offload->flow->timeout,
> 				       lastused + flow_offload_get_timeout(offload->flow));
> }
> 
> and then in the shared epilogue:
> 
> 	clear_bit(NF_FLOW_HW_PENDING, &offload->flow->flags);
> 
> That looks like the same "worker writes flow flags after the flow may have
> been freed" pattern the commit message describes, just reached through the
> stats work item instead of the destroy one.
> 
> The ordering in nf_flow_table_gc_cleanup() seems to make this explicit on the
> netdev-down and flowtable-delete paths:
> 
> net/netfilter/nf_flow_table_core.c:nf_flow_table_gc_cleanup() {
> 	nf_flow_table_iterate(flowtable, nf_flow_table_do_cleanup, dev);
> 	flush_delayed_work(&flowtable->gc_work);
> 	nf_flow_table_offload_flush(flowtable);
> }
> 
> Teardown is set, the gc flush frees the HW_DEAD flows, and only afterwards
> are the add/del/stats workqueues drained.  Should the flush of the offload
> workqueues happen before the gc flush here?
> 
> Would it make sense for the free decision in nf_flow_offload_gc_step() to
> also consult NF_FLOW_HW_PENDING, or for the work items to take a reference
> on the flow (or for work admission to be refused once NF_FLOW_HW_DYING is
> set), rather than only reordering the two bit operations in the destroy
> worker?
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918112844.194503-2-pablo%40netfilter.org

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net 8/8] netfilter: nf_tables: skip expired catchall elements on insert and delete
  2026-09-19 13:34   ` netdev-bot+sashiko
@ 2026-09-19 14:57     ` Pablo Neira Ayuso
  0 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-19 14:57 UTC (permalink / raw)
  To: kuba
  Cc: netdev-bot+sashiko, netfilter-devel, davem, netdev, pabeni,
	edumazet, horms, fw, ja

Hi Jakub,

This comments below look like brainstorming, nothing really that
should prevent this patch from being merged.

On Sat, Sep 19, 2026 at 01:34:51PM +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
> 
> - [Medium] The relaxed duplicate predicate in
>   nft_setelem_catchall_insert() (nf_tables_api.c:7003-7016) removes the…
> 
> --
> 
> netfilter: nf_tables: skip expired catchall elements on insert and delete
> 
> This aligns nft_setelem_catchall_insert() and
> nft_setelem_catchall_deactivate() with the lookup and dump predicates, so
> an expired catchall element no longer counts as a duplicate and no longer
> gets picked as the delete candidate. Both walks now also test
> __nft_set_elem_expired() with the per-netns transaction timestamp and
> nft_set_elem_is_dead().
> 
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index c0b754a2d45b..b59628e6240c 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -6995,11 +6995,14 @@ static int nft_setelem_catchall_insert(const struct net *net,
> >  {
> >  	struct nft_set_elem_catchall *catchall;
> >  	u8 genmask = nft_genmask_next(net);
> > +	u64 tstamp = nft_net_tstamp(net);
> >  	struct nft_set_ext *ext;
> >  
> >  	list_for_each_entry(catchall, &set->catchall_list, list) {
> >  		ext = nft_set_elem_ext(set, catchall->elem);
> > -		if (nft_set_elem_active(ext, genmask)) {
> > +		if (nft_set_elem_active(ext, genmask) &&
> > +		    !__nft_set_elem_expired(ext, tstamp) &&
> > +		    !nft_set_elem_is_dead(ext)) {
> >  			*priv = catchall->elem;
> >  			return -EEXIST;
> >  		}
> >  	}
> 
> [Severity: Medium]
> Can repeated expire-then-readd cycles grow set->catchall_list without
> bound here?
> 
> With the relaxed predicate, the expired entry is skipped but stays linked
> and genmask-active until GC runs, so the tail of
> nft_setelem_catchall_insert() appends a second entry alongside it:
> 
> 	catchall = kmalloc_obj(*catchall, GFP_KERNEL_ACCOUNT);
> 	if (!catchall)
> 		return -ENOMEM;
> 
> 	catchall->elem = elem->priv;
> 	list_add_tail_rcu(&catchall->list, &set->catchall_list);
> 
> Unlike ordinary elements, catchall elements are excluded from the size
> accounting in nft_add_set_elem():

Set size is specified by user... User can create a set without a set
size... This comment makes no sense.

> 	if (!(flags & NFT_SET_ELEM_CATCHALL)) {
> 		unsigned int max = nft_set_maxsize(set), nelems;
> 
> 		nelems = atomic_inc_return(&set->nelems);
> 		if (nelems > max)
> 			set_full = true;
> 	}
> 
> so the "return set_full ? -ENFILE : 0;" limiter that caps accumulation of
> ordinary elements does not apply to this path. Is there anything else that
> bounds the number of stale catchall entries a set can hold?

We could sets a cap for maximum number of items in a transaction, but
noone requested this so far. There is also memcg which should provide
a cap.

> Reclamation looks like it depends entirely on GC, and the interval is
> user-tunable with no upper bound (NFTA_SET_GC_INTERVAL is accepted as a u32
> in milliseconds and nft_set_gc_interval() just converts it):
> 
> include/net/netfilter/nf_tables.h:nft_set_gc_interval() {
> 	u32 gc_int = READ_ONCE(set->gc_int);
> 
> 	return gc_int ? msecs_to_jiffies(gc_int) : HZ;
> }
> 
> For nft_set_rhash and nft_set_rbtree the catchall reclaim runs only from
> the delayed gc work requeued with that value, so with a large gc-interval
> the window for accumulation is correspondingly large.
>
> There is also a packet-path effect: nft_set_catchall_lookup() walks the
> whole list, and new elements are appended at the tail, so every accumulated
> stale entry is dereferenced before the live one is found:
> 
> 	list_for_each_entry_rcu(catchall, &set->catchall_list, list) {
> 		ext = nft_set_elem_ext(set, catchall->elem);
> 		if (nft_set_elem_active(ext, genmask) &&
> 		    !nft_set_elem_expired(ext) &&
> 		    !nft_set_elem_is_dead(ext))
> 			return ext;
> 	}
> 
> Would it be preferable to reuse or force-reclaim the stale catchall element
> on insert, rather than only ignoring it?

Maybe, but that is unrelated to this patch.

> > @@ -7092,11 +7095,14 @@ static int nft_setelem_catchall_deactivate(const struct net *net,
> >  					   struct nft_set_elem *elem)
> >  {
> >  	struct nft_set_elem_catchall *catchall;
> > +	u64 tstamp = nft_net_tstamp(net);
> >  	struct nft_set_ext *ext;
> >  
> >  	list_for_each_entry(catchall, &set->catchall_list, list) {
> >  		ext = nft_set_elem_ext(set, catchall->elem);
> > -		if (!nft_is_active_next(net, ext))
> > +		if (!nft_is_active_next(net, ext) ||
> > +		    __nft_set_elem_expired(ext, tstamp) ||
> > +		    nft_set_elem_is_dead(ext))
> >  			continue;
> >  
> >  		kfree(elem->priv);
> 
> [Severity: Medium]
> Related to the above: with stale entries now skipped and -ENOENT returned,
> per-element DELSETELEM can no longer retire the accumulated entries, so a
> full set flush or set deletion appears to be the only way to drop them
> before GC fires. Is that the intended behaviour?
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918112844.194503-2-pablo%40netfilter.org

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net 4/8] netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read
  2026-09-19 13:34   ` netdev-bot+sashiko
@ 2026-09-19 15:02     ` Pablo Neira Ayuso
  0 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2026-09-19 15:02 UTC (permalink / raw)
  To: kuba
  Cc: netdev-bot+sashiko, netfilter-devel, davem, netdev, kuba, pabeni,
	edumazet, horms, fw, ja

Hi Jakub,

IP6T_RT_FST_MASK is just fine.

On Sat, Sep 19, 2026 at 01:34:49PM +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: 0 · Low: 1
> 
> - [Low] Validator/matcher flag-semantics disagreement in
>   net/ipv6/netfilter/ip6t_rt.c.
> 
> --
> 
> netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read
> 
> This adds a loop-header bound (i < rtinfo->addrnr) to the non-strict
> address matching loop in rt_mt6(), removing the backwards break at the
> bottom of the loop.  It also adds a rt_mt6_check() rejection for rules
> that set IP6T_RT_FST_MASK with rtinfo->addrnr == 0.
> 
> > diff --git a/net/ipv6/netfilter/ip6t_rt.c b/net/ipv6/netfilter/ip6t_rt.c
> > index 8051425213dd..9880faf3cc7d 100644
> > --- a/net/ipv6/netfilter/ip6t_rt.c
> > +++ b/net/ipv6/netfilter/ip6t_rt.c
> > @@ -96,7 +96,8 @@ static bool rt_mt6(const struct sk_buff *skb, struct xt_action_param *par)
> >  			unsigned int i = 0;
> >  
> >  			for (temp = 0;
> > -			     temp < (unsigned int)((hdrlen - 8) / 16);
> > +			     temp < (unsigned int)((hdrlen - 8) / 16) &&
> > +			     i < rtinfo->addrnr;
> >  			     temp++) {
> >  				ap = skb_header_pointer(skb,
> >  							ptr
> > @@ -112,8 +113,6 @@ static bool rt_mt6(const struct sk_buff *skb, struct xt_action_param *par)
> >  
> >  				if (ipv6_addr_equal(ap, &rtinfo->addrs[i]))
> >  					i++;
> > -				if (i == rtinfo->addrnr)
> > -					break;
> >  			}
> >  			if (i == rtinfo->addrnr)
> >  				return ret;
> 
> [ ... ]
> 
> > @@ -162,6 +161,12 @@ static int rt_mt6_check(const struct xt_mtchk_param *par)
> >  		pr_info_ratelimited("too many addresses specified\n");
> >  		return -EINVAL;
> >  	}
> > +
> > +	if ((rtinfo->flags & IP6T_RT_FST_MASK) && !rtinfo->addrnr) {
> > +		pr_info_ratelimited("address list match requested but addrnr is 0\n");
> > +		return -EINVAL;
> > +	}
> > +
> 
> [Severity: Low]
> Is the mask used here wider than the flag the matcher actually keys on?
> IP6T_RT_FST_MASK is 0x30, covering both bits:
> 
> include/uapi/linux/netfilter_ipv6/ip6t_rt.h:
>     #define IP6T_RT_FST_MASK	0x30
>     #define IP6T_RT_FST 		0x10
>     #define IP6T_RT_FST_NSTRICT	0x20

This is from iptables:

        case O_RT0NSTRICT:
                if (!(cb->xflags & F_RT0ADDRS))
                        xtables_error(PARAMETER_PROBLEM,
                                   "`--rt-0-addr ...' required before `--rt-0-not-strict'");
                rtinfo->flags |= IP6T_RT_FST_NSTRICT;

The error message says that --rt-0-not-strict needs --rt-0-addr,
which is IP6T_RT_FST.

... that is IP6T_RT_FST_MASK.

> But rt_mt6() gates every access to rtinfo->addrs[] and rtinfo->addrnr on
> IP6T_RT_FST alone:
> 
> net/ipv6/netfilter/ip6t_rt.c:rt_mt6() {
> 	if (!(rtinfo->flags & IP6T_RT_FST)) {
> 		return ret;
> 	} else if (rtinfo->flags & IP6T_RT_FST_NSTRICT) {
> 	...
> }
> 
> So a rule with only IP6T_RT_FST_NSTRICT set and addrnr == 0 never touches
> addrs[] at match time, yet it is now rejected with -EINVAL at load time.

Yes, but IP6T_RT_FST_NSTRICT set and addrnr == 0 should not ever
happen.

This LLM is lacking userspace context.

> Would narrowing the new test to IP6T_RT_FST avoid that load-time behaviour
> change?  Such blobs can only come from direct IP6T_SO_SET_REPLACE or
> nft_compat callers, since libip6t_rt requires --rt-0-addrs before
> --rt-0-not-strict, so no ruleset produced by iptables appears to be
> affected.
> 
> Does the new check also make the strict branch's addrnr == 0 case
> unreachable?  That branch has a meaningful zero-address semantic, matching
> a routing header that carries no addresses at all (hdrlen == 8):
> 
> net/ipv6/netfilter/ip6t_rt.c:rt_mt6() {
> 	...
> 			if (temp == rtinfo->addrnr &&
> 			    temp == (unsigned int)((hdrlen - 8) / 16))
> 				return ret;
> 	...
> }
> 
> With addrnr == 0 now refused at load time, that comparison can no longer be
> satisfied with addrnr == 0.
> 
> One more thing worth noting: is the new checkentry test needed for memory
> safety at all?  The loop-header change bounds i to addrnr, and addrnr is
> already capped at IP6T_RT_HOPS by the preceding test, so &rtinfo->addrs[i]
> stays inside the 16-entry array on its own.
> 
> Separately, and not something this patch introduces: a rule with
> IP6T_RT_TYP | IP6T_RT_FST_NSTRICT, rt_type 0 and addrnr > 0 still passes
> rt_mt6_check() as an address list match, but rt_mt6() returns at the
> !(rtinfo->flags & IP6T_RT_FST) test and silently ignores every configured
> address.  Should the validator and the matcher agree on which flag means
> "match the address list"?
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918112844.194503-2-pablo%40netfilter.org

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net 1/8] netfilter: flowtable: publish HW_DEAD after worker is done
  2026-09-18 11:28 ` [PATCH net 1/8] netfilter: flowtable: publish HW_DEAD after worker is done Pablo Neira Ayuso
  2026-09-19 13:34   ` netdev-bot+sashiko
@ 2026-09-21 22:20   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 16+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-21 22:20 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netfilter-devel, davem, netdev, kuba, pabeni, edumazet, horms, fw,
	ja

Hello:

This series was applied to netdev/net.git (main)
by Pablo Neira Ayuso <pablo@netfilter.org>:

On Fri, 18 Sep 2026 13:28:37 +0200 you wrote:
> From: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> 
> flow_offload_work_del() sets NF_FLOW_HW_DEAD before the work handler
> clears NF_FLOW_HW_PENDING. Once a flow is both HW_DYING and HW_DEAD, a
> concurrent garbage collection pass can remove it and schedule it for RCU
> freeing.
> 
> [...]

Here is the summary with links:
  - [net,1/8] netfilter: flowtable: publish HW_DEAD after worker is done
    https://git.kernel.org/netdev/net/c/d644b23afe1e
  - [net,2/8] netfilter: nfnetlink_queue: hold nfnl mutex in event notifier
    https://git.kernel.org/netdev/net/c/9461613afc59
  - [net,3/8] netfilter: ip6t_rpfilter: reject routes without inet6_dev
    https://git.kernel.org/netdev/net/c/1b9b5323725e
  - [net,4/8] netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read
    https://git.kernel.org/netdev/net/c/82313c169edd
  - [net,5/8] netfilter: nft_synproxy: use the family-aware checksum helper
    https://git.kernel.org/netdev/net/c/a311a8981727
  - [net,6/8] ipvs: revalidate ihl before icmp_send
    https://git.kernel.org/netdev/net/c/e29014556488
  - [net,7/8] netfilter: ctnetlink: fix suspicious RCU usage in expect_iter_name
    https://git.kernel.org/netdev/net/c/207d591c3532
  - [net,8/8] netfilter: nf_tables: skip expired catchall elements on insert and delete
    https://git.kernel.org/netdev/net/c/70194dc37670

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-09-21 22:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-18 11:28 [PATCH net,v2 0/8] Netfilter/IPVS fixes for net Pablo Neira Ayuso
2026-09-18 11:28 ` [PATCH net 1/8] netfilter: flowtable: publish HW_DEAD after worker is done Pablo Neira Ayuso
2026-09-19 13:34   ` netdev-bot+sashiko
2026-09-19 14:47     ` Pablo Neira Ayuso
2026-09-21 22:20   ` patchwork-bot+netdevbpf
2026-09-18 11:28 ` [PATCH net 2/8] netfilter: nfnetlink_queue: hold nfnl mutex in event notifier Pablo Neira Ayuso
2026-09-18 11:28 ` [PATCH net 3/8] netfilter: ip6t_rpfilter: reject routes without inet6_dev Pablo Neira Ayuso
2026-09-18 11:28 ` [PATCH net 4/8] netfilter: ip6t_rt: fix zero-address non-strict match out-of-bounds read Pablo Neira Ayuso
2026-09-19 13:34   ` netdev-bot+sashiko
2026-09-19 15:02     ` Pablo Neira Ayuso
2026-09-18 11:28 ` [PATCH net 5/8] netfilter: nft_synproxy: use the family-aware checksum helper Pablo Neira Ayuso
2026-09-18 11:28 ` [PATCH net 6/8] ipvs: revalidate ihl before icmp_send Pablo Neira Ayuso
2026-09-18 11:28 ` [PATCH net 7/8] netfilter: ctnetlink: fix suspicious RCU usage in expect_iter_name Pablo Neira Ayuso
2026-09-18 11:28 ` [PATCH net 8/8] netfilter: nf_tables: skip expired catchall elements on insert and delete Pablo Neira Ayuso
2026-09-19 13:34   ` netdev-bot+sashiko
2026-09-19 14:57     ` Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).