Re: [PATCH stable 6.12 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:
- [High] The backport fails to apply the `pinet6` initialization fix to the DCCP subsystem, leaving DCCP IPv4-mapped IPv6 sockets vulnerable to the original race condition.
- [High] Listener's IPv6 lists and options are incorrectly freed if routing fails for a v6-mapped TCP or DCCP child socket, due to a stale `pinet6` pointer.
--

commit e1ad03aa81aeb31e12241a45ce49d5481393aa47
Author: Eric Dumazet <edumazet@google.com>

tcp: fix potential race in tcp_v6_syn_recv_sock()

The commit moves the initialization of the IPv6 state for an IPv4-mapped IPv6
child socket to before the socket is inserted into the ehash table. This fixes
a race condition where other CPUs could access the partially initialized socket.

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

> diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
> index 5926159a6f204..b8f5e20368ec5 100644
> --- a/net/dccp/ipv4.c
> +++ b/net/dccp/ipv4.c
> @@ -410,7 +410,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: High]
This isn't a bug introduced by this patch, but does this backport completely
fix the race condition for DCCP?

The `opt_child_init` parameter is accepted in `dccp_v4_request_recv_sock()` but
is never executed. When `inet_ehash_nolisten()` is called later in this
function, the socket becomes visible to other CPUs.

> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
> index d6649246188d7..c1db853667479 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
[ ... ]
> @@ -434,7 +436,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: High]
This is a pre-existing issue, but DCCP still performs the IPv6 initialization
manually after `dccp_v4_request_recv_sock()` returns and the socket is already
in the ehash table.

Could this leave DCCP IPv4-mapped IPv6 sockets vulnerable to the same race
condition where another CPU accesses the child socket's `pinet6` concurrently?

> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 5afe5e57c89b5..550d64babb94e 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
[ ... ]
> @@ -1796,6 +1798,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: High]
This is a pre-existing issue, but does placing `opt_child_init` here leave the
early routing failure path exposed to memory corruption?

Earlier in `tcp_v4_syn_recv_sock()`, `inet_csk_route_child_sock()` can fail
(e.g., due to an unroutable spoofed source IP) and jump to the `put_and_exit`
label.

Because `opt_child_init` has not yet been called, `pinet6` still points to the
listener's `ipv6_pinfo`. When `tcp_done(newsk)` is subsequently called in the
error path, it invokes the IPv6 socket destructor. Will this incorrectly free
the listener's IPv6 lists and options, leading to a resource leak or crash?

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