All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] tcp: initialize mapped children before route lookup
@ 2026-10-28 15:41 Eric Dumazet
  2026-10-28 15:45 ` netdev-bot+sinfo
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Eric Dumazet @ 2026-10-28 15:41 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
	Neal Cardwell
  Cc: Simon Horman, netdev, Kyle Zeng, Eric Dumazet

From: Kyle Zeng <kylebot@openai.com>

IPv4 passive opens on a dual-stack listener go through
tcp_v4_syn_recv_sock() with tcp_v6_mapped_child_init() as the optional
initializer. The callback currently runs after the output route lookup.
If that lookup fails, tcp_done() destroys a clone whose pinet6 and
ipv6_fl_list still refer to the listener's state.

With the flowlabel list head in inet_sock, fl6_free_socklist() walks
the clone's list and RCU-frees its nodes without unlinking them from the
live listener. Subsequent flowlabel operations can use or free the nodes
again. Commit 858d2a4f67ff ("tcp: fix potential race in
tcp_v6_syn_recv_sock()") moved the initializer before ehash insertion,
but did not cover this error path.

Invoke the existing initializer after the IPv4 fields it consumes have
been set, but before route lookup can fail. It needs neither the
destination nor sk_setup_caps(), and all later disposal paths remain
covered.

Fixes: 1c17f4373d4d ("ipv6: Move ipv6_fl_list from ipv6_pinfo to inet_sock.")
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/ipv4/tcp_ipv4.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 3f8563a36db1e9ea853699ecb547af7ee9491b11..c08de825fa448cfcd42c9a8b407200b0dc5b7b1d 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1723,6 +1723,12 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
 	if (READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_reflect_tos))
 		newinet->tos = tcp_rsk(req)->syn_tos & ~INET_ECN_MASK;
 
+#if IS_ENABLED(CONFIG_IPV6)
+	/* Detach IPv6 state before a route lookup can fail. */
+	if (opt_child_init)
+		opt_child_init(newsk, sk);
+#endif
+
 	if (!dst) {
 		dst = inet_csk_route_child_sock(sk, newsk, req);
 		if (!dst)
@@ -1732,10 +1738,6 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
 	}
 	sk_setup_caps(newsk, dst);
 
-#if IS_ENABLED(CONFIG_IPV6)
-	if (opt_child_init)
-		opt_child_init(newsk, sk);
-#endif
 	tcp_ca_openreq_child(newsk, dst);
 
 	tcp_sync_mss(newsk, dst4_mtu(dst));
-- 
2.53.0


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

* Re: [PATCH net] tcp: initialize mapped children before route lookup
  2026-10-28 15:41 [PATCH net] tcp: initialize mapped children before route lookup Eric Dumazet
@ 2026-10-28 15:45 ` netdev-bot+sinfo
  2026-10-28 19:58 ` Kuniyuki Iwashima
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sinfo @ 2026-10-28 15:45 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
	Neal Cardwell, Simon Horman, netdev, Kyle Zeng

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.

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

* Re: [PATCH net] tcp: initialize mapped children before route lookup
  2026-10-28 15:41 [PATCH net] tcp: initialize mapped children before route lookup Eric Dumazet
  2026-10-28 15:45 ` netdev-bot+sinfo
@ 2026-10-28 19:58 ` Kuniyuki Iwashima
  2026-10-29 16:05 ` netdev-bot+sashiko
  2026-10-29 16:20 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-28 19:58 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Neal Cardwell,
	Simon Horman, netdev, Kyle Zeng

On Wed, 28 Oct 2026 16:41:01 +0100, Eric Dumazet wrote:
> From: Kyle Zeng <kylebot@openai.com>
> 
> IPv4 passive opens on a dual-stack listener go through
> tcp_v4_syn_recv_sock() with tcp_v6_mapped_child_init() as the optional
> initializer. The callback currently runs after the output route lookup.
> If that lookup fails, tcp_done() destroys a clone whose pinet6 and
> ipv6_fl_list still refer to the listener's state.
> 
> With the flowlabel list head in inet_sock, fl6_free_socklist() walks
> the clone's list and RCU-frees its nodes without unlinking them from the
> live listener. Subsequent flowlabel operations can use or free the nodes
> again. Commit 858d2a4f67ff ("tcp: fix potential race in
> tcp_v6_syn_recv_sock()") moved the initializer before ehash insertion,
> but did not cover this error path.
> 
> Invoke the existing initializer after the IPv4 fields it consumes have
> been set, but before route lookup can fail. It needs neither the
> destination nor sk_setup_caps(), and all later disposal paths remain
> covered.
> 
> Fixes: 1c17f4373d4d ("ipv6: Move ipv6_fl_list from ipv6_pinfo to inet_sock.")
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Thanks!

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

* Re: [PATCH net] tcp: initialize mapped children before route lookup
  2026-10-28 15:41 [PATCH net] tcp: initialize mapped children before route lookup Eric Dumazet
  2026-10-28 15:45 ` netdev-bot+sinfo
  2026-10-28 19:58 ` Kuniyuki Iwashima
@ 2026-10-29 16:05 ` netdev-bot+sashiko
  2026-10-29 16:20 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-10-29 16:05 UTC (permalink / raw)
  To: edumazet; +Cc: davem, kuba, pabeni, kuniyu, ncardwell, horms, netdev, kylebot

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 code change is correct, but the commit message understates
  what it fixes and the Fixes: tag is too narrow for stable backports.

--

tcp: initialize mapped children before route lookup

This patch moves the opt_child_init() call in tcp_v4_syn_recv_sock() so it
runs before inet_csk_route_child_sock(). A v4-mapped child on a dual-stack
listener now gets its pinet6 and ipv6_fl_list detached from the listener
before the route lookup can fail and tcp_done() destroys the clone.

> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index ebe5aa70374a3..a7f9242f1e429 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1723,6 +1723,12 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
>  	if (READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_reflect_tos))
>  		newinet->tos = tcp_rsk(req)->syn_tos & ~INET_ECN_MASK;
>  
> +#if IS_ENABLED(CONFIG_IPV6)
> +	/* Detach IPv6 state before a route lookup can fail. */
> +	if (opt_child_init)
> +		opt_child_init(newsk, sk);
> +#endif
> +
>  	if (!dst) {
>  		dst = inet_csk_route_child_sock(sk, newsk, req);
>  		if (!dst)

[Severity: Medium]

The code change looks right. Does the commit message undersell what this
fixes, though? And is the Fixes: tag narrower than it should be?

The commit message only describes the flowlabel use-after-free:

  With the flowlabel list head in inet_sock, fl6_free_socklist() walks
  the clone's list and RCU-frees its nodes without unlinking them from the
  live listener.

  Fixes: 1c17f4373d4d ("ipv6: Move ipv6_fl_list from ipv6_pinfo to inet_sock.")

Before this patch, if inet_csk_route_child_sock() fails, the clone made by
sk_clone_lock() still has newinet->pinet6 pointing at the listener's
ipv6_pinfo. Only tcp_v6_mapped_child_init() resets it:

  newinet->pinet6 = newnp = tcp_inet6_sk(newsk);
  newinet->ipv6_fl_list = NULL;

The clone is then torn down like this:

tcp_v4_syn_recv_sock()
  put_and_exit
    inet_csk_prepare_forced_close()
    tcp_done(newsk)
      ... __sk_destruct()
        inet6_sock_destruct()
          inet6_cleanup_sock()

In that path, inet6_cleanup_sock() uses np = inet6_sk() of the child, which
is the listener's pinfo:

net/ipv6/af_inet6.c:inet6_cleanup_sock() {
    ...
	skb = xchg(&np->pktoptions, NULL);
	kfree_skb(skb);

	skb = xchg(&np->rxpmtu, NULL);
	kfree_skb(skb);
    ...
	opt = unrcu_pointer(xchg(&np->opt, NULL));
	if (opt) {
		atomic_sub(opt->tot_len, &sk->sk_omem_alloc);
		txopt_put(opt);
	}
    ...
}

Wouldn't this free the listener's pktoptions and rxpmtu? It would also
drop the IPv6 tx options set on the listener with setsockopt (hop-by-hop,
routing and destination headers).

The atomic_sub() also seems to hit the child's sk_omem_alloc. That would
leave the listener charged for opt->tot_len and push the child's counter
negative.

None of this depends on 1c17f4373d4d. On trees before 1c17f4373d4d,
fl6_free_socklist() worked on np->ipv6_fl_list. The aliased np would
therefore drain and release the listener's own flowlabel list as well.

The aliasing on this error path seems to go back to the original v4-mapped
handling. Commit 858d2a4f67ff ("tcp: fix potential race in
tcp_v6_syn_recv_sock()"), which has Fixes: 1da177e4c3f4, added the
opt_child_init callback but left it after the route lookup. This patch
builds on that callback.

With only Fixes: 1c17f4373d4d, would stable trees that have 858d2a4f67ff
but not 1c17f4373d4d (older than v6.19) skip this fix? Those trees would
keep the listener state clobbering described above.

Could the commit message mention the np->opt, pktoptions and rxpmtu impact?
It could also add a Fixes: tag for 858d2a4f67ff (or 1da177e4c3f4), or list
the affected stable trees.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261028154101.2153360-1-edumazet%40kernel.org

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

* Re: [PATCH net] tcp: initialize mapped children before route lookup
  2026-10-28 15:41 [PATCH net] tcp: initialize mapped children before route lookup Eric Dumazet
                   ` (2 preceding siblings ...)
  2026-10-29 16:05 ` netdev-bot+sashiko
@ 2026-10-29 16:20 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-29 16:20 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: davem, kuba, pabeni, kuniyu, ncardwell, horms, netdev, kylebot

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 28 Oct 2026 16:41:01 +0100 you wrote:
> From: Kyle Zeng <kylebot@openai.com>
> 
> IPv4 passive opens on a dual-stack listener go through
> tcp_v4_syn_recv_sock() with tcp_v6_mapped_child_init() as the optional
> initializer. The callback currently runs after the output route lookup.
> If that lookup fails, tcp_done() destroys a clone whose pinet6 and
> ipv6_fl_list still refer to the listener's state.
> 
> [...]

Here is the summary with links:
  - [net] tcp: initialize mapped children before route lookup
    https://git.kernel.org/netdev/net/c/b7ff0e7a9ae3

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] 5+ messages in thread

end of thread, other threads:[~2026-10-29 16:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 15:41 [PATCH net] tcp: initialize mapped children before route lookup Eric Dumazet
2026-10-28 15:45 ` netdev-bot+sinfo
2026-10-28 19:58 ` Kuniyuki Iwashima
2026-10-29 16:05 ` netdev-bot+sashiko
2026-10-29 16:20 ` patchwork-bot+netdevbpf

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.