All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH nf] netfilter: nft_reject: prevent unbounded recursion in output hooks
@ 2026-09-21 22:29 Fernando Fernandez Mancera
  2026-09-22  6:18 ` Florian Westphal
  0 siblings, 1 reply; 3+ messages in thread
From: Fernando Fernandez Mancera @ 2026-09-21 22:29 UTC (permalink / raw)
  To: netfilter-devel
  Cc: coreteam, pablo, fw, phil, eric, Fernando Fernandez Mancera,
	Yu Junzhe

nftables payload-mangling rules can rewrite packet contents (such as TCP
flags) before they are evaluated by reject targets. This defeats the
content-based loop guards, allowing unbounded hook recursion that leads
to kernel stack exhaustion and panics.

Fix this by extracting the per-CPU xmit recursion limit previously used
by the netdev duplication/forwarding paths into a shared header, and
apply it to the inet and inet6 reject transmit functions. It disables BH
while transmitting the skb to address a possible migration to different
CPU leading to imbalanced decrementation of the recursion counters.

Note that the dev_queue_xmit() path used when CONFIG_BRIDGE_NETFILTER is
enabled does not require this recursion guard. In that scenario, the
generated reset packet is transmitted directly at Layer 2. Because the
packet egresses from a different hook, it cannot synchronously loop.

Reported-by: Yu Junzhe <junzheyu1@gmail.com>
Closes: https://lore.kernel.org/netfilter-devel/20260921061320.543-1-junzheyu1@gmail.com/
Fixes: bee11dc78fc8 ("netfilter: nft_reject: support for IPv6 and TCP reset")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 include/net/netfilter/nf_dup_netdev.h | 35 ----------------------
 include/net/netfilter/nf_recursion.h  | 42 +++++++++++++++++++++++++++
 net/ipv4/netfilter/nf_reject_ipv4.c   | 12 ++++++++
 net/ipv6/netfilter/nf_reject_ipv6.c   | 13 +++++++++
 net/netfilter/nf_dup_netdev.c         |  1 +
 net/netfilter/nft_fwd_netdev.c        |  1 +
 6 files changed, 69 insertions(+), 35 deletions(-)
 create mode 100644 include/net/netfilter/nf_recursion.h

diff --git a/include/net/netfilter/nf_dup_netdev.h b/include/net/netfilter/nf_dup_netdev.h
index f6b05bd80c3f..a39411264614 100644
--- a/include/net/netfilter/nf_dup_netdev.h
+++ b/include/net/netfilter/nf_dup_netdev.h
@@ -9,41 +9,6 @@
 void nf_dup_netdev_egress(const struct nft_pktinfo *pkt, int oif);
 void nf_fwd_netdev_egress(const struct nft_pktinfo *pkt, int oif);
 
-#define NF_RECURSION_LIMIT	2
-
-#ifndef CONFIG_PREEMPT_RT
-static inline bool nf_dev_xmit_recursion(void)
-{
-	return unlikely(__this_cpu_read(softnet_data.xmit.nf_dup_skb_recursion) >
-			NF_RECURSION_LIMIT);
-}
-
-static inline void nf_dev_xmit_recursion_inc(void)
-{
-	__this_cpu_inc(softnet_data.xmit.nf_dup_skb_recursion);
-}
-
-static inline void nf_dev_xmit_recursion_dec(void)
-{
-	__this_cpu_dec(softnet_data.xmit.nf_dup_skb_recursion);
-}
-#else
-static inline bool nf_dev_xmit_recursion(void)
-{
-	return unlikely(current->net_xmit.nf_dup_skb_recursion > NF_RECURSION_LIMIT);
-}
-
-static inline void nf_dev_xmit_recursion_inc(void)
-{
-	current->net_xmit.nf_dup_skb_recursion++;
-}
-
-static inline void nf_dev_xmit_recursion_dec(void)
-{
-	current->net_xmit.nf_dup_skb_recursion--;
-}
-#endif
-
 struct nft_offload_ctx;
 struct nft_flow_rule;
 
diff --git a/include/net/netfilter/nf_recursion.h b/include/net/netfilter/nf_recursion.h
new file mode 100644
index 000000000000..3e91b78e21b3
--- /dev/null
+++ b/include/net/netfilter/nf_recursion.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _NF_RECURSION_H_
+#define _NF_RECURSION_H_
+
+#include <linux/netdevice.h>
+
+#define NF_RECURSION_LIMIT	2
+
+#ifndef CONFIG_PREEMPT_RT
+static inline bool nf_dev_xmit_recursion(void)
+{
+	return unlikely(__this_cpu_read(softnet_data.xmit.nf_dup_skb_recursion) >
+			NF_RECURSION_LIMIT);
+}
+
+static inline void nf_dev_xmit_recursion_inc(void)
+{
+	__this_cpu_inc(softnet_data.xmit.nf_dup_skb_recursion);
+}
+
+static inline void nf_dev_xmit_recursion_dec(void)
+{
+	__this_cpu_dec(softnet_data.xmit.nf_dup_skb_recursion);
+}
+#else
+static inline bool nf_dev_xmit_recursion(void)
+{
+	return unlikely(current->net_xmit.nf_dup_skb_recursion > NF_RECURSION_LIMIT);
+}
+
+static inline void nf_dev_xmit_recursion_inc(void)
+{
+	current->net_xmit.nf_dup_skb_recursion++;
+}
+
+static inline void nf_dev_xmit_recursion_dec(void)
+{
+	current->net_xmit.nf_dup_skb_recursion--;
+}
+#endif
+
+#endif /* _NF_RECURSION_H_ */
diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
index 59ec465a9df9..3f227a094312 100644
--- a/net/ipv4/netfilter/nf_reject_ipv4.c
+++ b/net/ipv4/netfilter/nf_reject_ipv4.c
@@ -10,6 +10,7 @@
 #include <net/dst.h>
 #include <net/dst_metadata.h>
 #include <net/netfilter/ipv4/nf_reject.h>
+#include <net/netfilter/nf_recursion.h>
 #include <linux/netfilter_ipv4.h>
 #include <linux/netfilter_bridge.h>
 
@@ -336,7 +337,18 @@ void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,
 		dev_queue_xmit(nskb);
 	} else
 #endif
+	{
+		local_bh_disable();
+		if (nf_dev_xmit_recursion()) {
+			local_bh_enable();
+			goto free_nskb;
+		}
+
+		nf_dev_xmit_recursion_inc();
 		ip_local_out(net, nskb->sk, nskb);
+		nf_dev_xmit_recursion_dec();
+		local_bh_enable();
+	}
 
 	return;
 
diff --git a/net/ipv6/netfilter/nf_reject_ipv6.c b/net/ipv6/netfilter/nf_reject_ipv6.c
index 07cdaa10da0d..f69ddfe72030 100644
--- a/net/ipv6/netfilter/nf_reject_ipv6.c
+++ b/net/ipv6/netfilter/nf_reject_ipv6.c
@@ -10,6 +10,7 @@
 #include <net/ip6_checksum.h>
 #include <net/dst_metadata.h>
 #include <net/netfilter/ipv6/nf_reject.h>
+#include <net/netfilter/nf_recursion.h>
 #include <linux/netfilter_ipv6.h>
 #include <linux/netfilter_bridge.h>
 
@@ -409,7 +410,19 @@ void nf_send_reset6(struct net *net, struct sock *sk, struct sk_buff *oldskb,
 		dev_queue_xmit(nskb);
 	} else
 #endif
+	{
+		local_bh_disable();
+		if (nf_dev_xmit_recursion()) {
+			local_bh_enable();
+			kfree_skb(nskb);
+			return;
+		}
+
+		nf_dev_xmit_recursion_inc();
 		ip6_local_out(net, sk, nskb);
+		nf_dev_xmit_recursion_dec();
+		local_bh_enable();
+	}
 }
 EXPORT_SYMBOL_GPL(nf_send_reset6);
 
diff --git a/net/netfilter/nf_dup_netdev.c b/net/netfilter/nf_dup_netdev.c
index c6bd5c29bed6..918716629c15 100644
--- a/net/netfilter/nf_dup_netdev.c
+++ b/net/netfilter/nf_dup_netdev.c
@@ -12,6 +12,7 @@
 #include <net/netfilter/nf_tables.h>
 #include <net/netfilter/nf_tables_offload.h>
 #include <net/netfilter/nf_dup_netdev.h>
+#include <net/netfilter/nf_recursion.h>
 
 static void nf_do_netdev_egress(struct sk_buff *skb, struct net_device *dev,
 				enum nf_dev_hooks hook)
diff --git a/net/netfilter/nft_fwd_netdev.c b/net/netfilter/nft_fwd_netdev.c
index a48c2f765bba..7447da931a95 100644
--- a/net/netfilter/nft_fwd_netdev.c
+++ b/net/netfilter/nft_fwd_netdev.c
@@ -14,6 +14,7 @@
 #include <net/netfilter/nf_tables.h>
 #include <net/netfilter/nf_tables_offload.h>
 #include <net/netfilter/nf_dup_netdev.h>
+#include <net/netfilter/nf_recursion.h>
 #include <net/neighbour.h>
 #include <net/ip.h>
 
-- 
2.55.0


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

* Re: [PATCH nf] netfilter: nft_reject: prevent unbounded recursion in output hooks
  2026-09-21 22:29 [PATCH nf] netfilter: nft_reject: prevent unbounded recursion in output hooks Fernando Fernandez Mancera
@ 2026-09-22  6:18 ` Florian Westphal
  2026-09-22  9:05   ` Fernando Fernandez Mancera
  0 siblings, 1 reply; 3+ messages in thread
From: Florian Westphal @ 2026-09-22  6:18 UTC (permalink / raw)
  To: Fernando Fernandez Mancera
  Cc: netfilter-devel, coreteam, pablo, phil, eric, Yu Junzhe

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> nftables payload-mangling rules can rewrite packet contents (such as TCP
> flags) before they are evaluated by reject targets. This defeats the
> content-based loop guards, allowing unbounded hook recursion that leads
> to kernel stack exhaustion and panics.
> 
> Fix this by extracting the per-CPU xmit recursion limit previously used
> by the netdev duplication/forwarding paths into a shared header, and
> apply it to the inet and inet6 reject transmit functions. It disables BH
> while transmitting the skb to address a possible migration to different
> CPU leading to imbalanced decrementation of the recursion counters.
> 
> Note that the dev_queue_xmit() path used when CONFIG_BRIDGE_NETFILTER is
> enabled does not require this recursion guard. In that scenario, the
> generated reset packet is transmitted directly at Layer 2. Because the
> packet egresses from a different hook, it cannot synchronously loop.

Won't nf_send_unreach() / unrach6 need the same guard?

If not, please explain why those are safe, its not clear to me.

Patch looks good otherwise, thanks Fernando.

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

* Re: [PATCH nf] netfilter: nft_reject: prevent unbounded recursion in output hooks
  2026-09-22  6:18 ` Florian Westphal
@ 2026-09-22  9:05   ` Fernando Fernandez Mancera
  0 siblings, 0 replies; 3+ messages in thread
From: Fernando Fernandez Mancera @ 2026-09-22  9:05 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, coreteam, pablo, phil, eric, Yu Junzhe

On 9/22/26 8:18 AM, Florian Westphal wrote:
> Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>> nftables payload-mangling rules can rewrite packet contents (such as TCP
>> flags) before they are evaluated by reject targets. This defeats the
>> content-based loop guards, allowing unbounded hook recursion that leads
>> to kernel stack exhaustion and panics.
>>
>> Fix this by extracting the per-CPU xmit recursion limit previously used
>> by the netdev duplication/forwarding paths into a shared header, and
>> apply it to the inet and inet6 reject transmit functions. It disables BH
>> while transmitting the skb to address a possible migration to different
>> CPU leading to imbalanced decrementation of the recursion counters.
>>
>> Note that the dev_queue_xmit() path used when CONFIG_BRIDGE_NETFILTER is
>> enabled does not require this recursion guard. In that scenario, the
>> generated reset packet is transmitted directly at Layer 2. Because the
>> packet egresses from a different hook, it cannot synchronously loop.
> 
> Won't nf_send_unreach() / unrach6 need the same guard?
> 
> If not, please explain why those are safe, its not clear to me.

Hi Florian,

If I am not wrong, those functions are safe. I initially thought it was 
needed too but after failing to reproduce the issue in that path I 
noticed that both icmp_send() and icmp6_send() are protected for 
recursions by disabling BH and using a xmit lock 
icmp_xmit_lock()/icmpv6_xmit_lock().

I will send a V2 mentioning this in the commit message so it is documented.

> 
> Patch looks good otherwise, thanks Fernando.
> 

Thanks Florian!

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

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-21 22:29 [PATCH nf] netfilter: nft_reject: prevent unbounded recursion in output hooks Fernando Fernandez Mancera
2026-09-22  6:18 ` Florian Westphal
2026-09-22  9:05   ` Fernando Fernandez Mancera

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.