Re: [PATCH stable 5.10 v2] tcp: fix potential race in tcp_v6_syn_recv_sock()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The backport failed to implement the semantic fix for DCCP v6-mapped sockets, leaving them vulnerable to the exact race condition patched in TCP.
- [Critical] Routing failure error paths in `tcp_v4_syn_recv_sock` and `dccp_v4_request_recv_sock` cause a Use-After-Free of the listener's IPv6 options for IPv6-mapped connections.
--

commit 17f9bf2cd985844a777166a25415341fdcf87b54
Author: Eric Dumazet <edumazet@google.com>
tcp: fix potential race in tcp_v6_syn_recv_sock()

This commit addresses a race condition where a newly created TCP child socket
becomes globally visible in the ehash table before its IPv6 fields are properly
initialized. It introduces an opt_child_init callback to perform the setup
earlier in tcp_v4_syn_recv_sock(). The patch also extends the callback mechanism
to other subsystems like DCCP to resolve similar issues there.

Link: https://patch.msgid.link/20260217161205.2079883-1-edumazet@google.com

> diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
> index b0b6e6a4784e5..86042ef2f5f13 100644
> --- a/net/dccp/ipv4.c
> +++ b/net/dccp/ipv4.c
> @@ -395,7 +395,9 @@ struct sock *dccp_v4_request_recv_sock(const struct sock *sk,
>  				       struct request_sock *req,
>  				       struct dst_entry *dst,
>  				       struct request_sock *req_unhash,
> -				       bool *own_req)
> +				       bool *own_req,
> +				       void (*opt_child_init)(struct sock *newsk,
> +							      const struct sock *sk))
>  {
>  	struct inet_request_sock *ireq;
>  	struct inet_sock *newinet;

[Severity: Critical]
This is a pre-existing issue, but does adding the opt_child_init parameter
here without actually calling it inside dccp_v4_request_recv_sock() leave
the DCCP v6-mapped sockets vulnerable to the exact same race condition being
fixed in TCP?

The child socket is inserted into the ehash table with its pinet6 pointer
aliasing the listener's ipv6_pinfo. If another CPU accesses the socket
before initialization completes, can this lead to data corruption or a
use-after-free?

> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
> index 78ee1b5acf1f1..5cc46957b0d29 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
[ ... ]
> @@ -413,7 +415,8 @@ static struct sock *dccp_v6_request_recv_sock(const struct sock *sk,
>  		 *	v6 mapped
>  		 */
>  		newsk = dccp_v4_request_recv_sock(sk, skb, req, dst,
> -						  req_unhash, own_req);
> +						  req_unhash, own_req,
> +						  opt_child_init);
>  		if (newsk == NULL)
>  			return NULL;

[Severity: Critical]
This is a pre-existing issue, but because opt_child_init is ignored in
dccp_v4_request_recv_sock(), the manual reassignment of pinet6 that happens
after this call returns still occurs too late.

Since dccp_v4_request_recv_sock() makes the socket globally visible before
returning, doesn't this leave the regression completely unfixed for DCCP?

> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 595dcc3afac5c..e922c10a1caf6 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
[ ... ]
> @@ -1557,6 +1559,10 @@ 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, dst_mtu(dst));

[Severity: Critical]
This isn't a bug introduced by this patch, but if inet_csk_route_child_sock()
fails earlier in tcp_v4_syn_recv_sock(), the code jumps to the put_and_exit
label and bypasses this new opt_child_init call entirely.

At the put_and_exit label, tcp_done() is called on the child socket, which
then invokes inet6_destroy_sock(). Since newinet->pinet6 was never reassigned
by opt_child_init, it still incorrectly aliases the listener socket's memory:

inet6_destroy_sock()
    ...
    skb = xchg(&np->pktoptions, NULL);
    kfree_skb(skb);

    skb = xchg(&np->rxpmtu, NULL);
    kfree_skb(skb);

Does this erroneous state cause inet6_destroy_sock() to free the active
listener socket's pktoptions, rxpmtu, and opt structures, leading to a
use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904183500.3841094-1-florian.fainelli@broadcom.com?part=1