[PATCH v3 net] udp: do not use skb_release_head_state() before skb_attempt_defer_free()

[PATCH v3 net] udp: do not use skb_release_head_state() before skb_attempt_defer_free()

[Eric Dumazet]

Michal reported and bisected an issue after recent adoption
of skb_attempt_defer_free() in UDP.

The issue here is that skb_release_head_state() is called twice per skb,
one time from skb_consume_udp(), then a second time from skb_defer_free_flush()
and napi_consume_skb().

As Sabrina suggested, remove skb_release_head_state() call from
skb_consume_udp().

Add a DEBUG_NET_WARN_ON_ONCE(skb_nfct(skb)) in skb_attempt_defer_free()

Many thanks to Michal, Sabrina, Paolo and Florian for their help.

Fixes: 6471658dc66c ("udp: use skb_attempt_defer_free()")
Reported-and-bisected-by: Michal Kubecek <mkubecek@suse.cz>
Closes: https://lore.kernel.org/netdev/gpjh4lrotyephiqpuldtxxizrsg6job7cvhiqrw72saz2ubs3h@g6fgbvexgl3r/
Signed-off-by: Eric Dumazet <edumazet@google.com>
Tested-by: Michal Kubecek <mkubecek@suse.cz>
Cc: Sabrina Dubroca <sd@queasysnail.net>
Cc: Florian Westphal <fw@strlen.de>
---
v3: Florian suggested skb_attempt_defer_free() as a better place for the DEBUG_NET_WARN_ON_ONCE(skb_nfct(skb)),
    since it already had the skb_dst(skb) and skb->destructor checks.
v2: Adopted Sabrina suggestion.
    https://lore.kernel.org/netdev/20251014132917.2841932-1-edumazet@google.com/T/#u
v1: https://lore.kernel.org/netdev/aO3_hBg5expKNv6v@krikkit/T/#m8a88669b801d85f57b73710cdb0c8ee63854af11

 net/core/skbuff.c | 1 +
 net/ipv4/udp.c    | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index bc12790017b0b5c0be99f8fb9d362b3730fa4eb0..6be01454f262a2acbf3f5905498961d132442d2c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -7200,6 +7200,7 @@ nodefer:	kfree_skb_napi_cache(skb);
 
 	DEBUG_NET_WARN_ON_ONCE(skb_dst(skb));
 	DEBUG_NET_WARN_ON_ONCE(skb->destructor);
+	DEBUG_NET_WARN_ON_ONCE(skb_nfct(skb));
 
 	sdn = per_cpu_ptr(net_hotdata.skb_defer_nodes, cpu) + numa_node_id();
 
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 95241093b7f01b2dc31d9520b693f46400e545ff..30dfbf73729dad1e7b26c74a4791fe0a82682a6d 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1851,8 +1851,6 @@ void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len)
 		sk_peek_offset_bwd(sk, len);
 
 	if (!skb_shared(skb)) {
-		if (unlikely(udp_skb_has_head_state(skb)))
-			skb_release_head_state(skb);
 		skb_attempt_defer_free(skb);
 		return;
 	}
-- 
2.51.0.788.g6d19910ace-goog

Re: [PATCH v3 net] udp: do not use skb_release_head_state() before skb_attempt_defer_free()

[Sabrina Dubroca]

2025-10-15, 05:27:15 +0000, Eric Dumazet wrote:
> Michal reported and bisected an issue after recent adoption
> of skb_attempt_defer_free() in UDP.
> 
> The issue here is that skb_release_head_state() is called twice per skb,
> one time from skb_consume_udp(), then a second time from skb_defer_free_flush()
> and napi_consume_skb().
> 
> As Sabrina suggested, remove skb_release_head_state() call from
> skb_consume_udp().
> 
> Add a DEBUG_NET_WARN_ON_ONCE(skb_nfct(skb)) in skb_attempt_defer_free()
> 
> Many thanks to Michal, Sabrina, Paolo and Florian for their help.
> 
> Fixes: 6471658dc66c ("udp: use skb_attempt_defer_free()")
> Reported-and-bisected-by: Michal Kubecek <mkubecek@suse.cz>
> Closes: https://lore.kernel.org/netdev/gpjh4lrotyephiqpuldtxxizrsg6job7cvhiqrw72saz2ubs3h@g6fgbvexgl3r/
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Michal Kubecek <mkubecek@suse.cz>
> Cc: Sabrina Dubroca <sd@queasysnail.net>
> Cc: Florian Westphal <fw@strlen.de>

Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

Thanks Eric.

-- 
Sabrina

Re: [PATCH v3 net] udp: do not use skb_release_head_state() before skb_attempt_defer_free()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 15 Oct 2025 05:27:15 +0000 you wrote:
> Michal reported and bisected an issue after recent adoption
> of skb_attempt_defer_free() in UDP.
> 
> The issue here is that skb_release_head_state() is called twice per skb,
> one time from skb_consume_udp(), then a second time from skb_defer_free_flush()
> and napi_consume_skb().
> 
> [...]

Here is the summary with links:
  - [v3,net] udp: do not use skb_release_head_state() before skb_attempt_defer_free()
    https://git.kernel.org/netdev/net/c/6de1dec1c166

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