Re: [PATCH net] tcp: ipv6: keep request pktopts alive until request destruction

[Kuniyuki Iwashima <kuniyu@google.com>]

On Mon, 02 Nov 2026 06:12:18 +0000, Yingjie Wang wrote:
> When IPv6 flow label reflection is enabled, tcp_v6_send_synack() reads
> ireq->pktopts to copy the flow label from the initial SYN.
> 
> Request sockets can be processed without the listener lock. A
> retransmitted SYN can therefore run tcp_v6_send_synack() while a
> concurrent final ACK creates the child in tcp_v6_syn_recv_sock(). The
> latter clones ireq->pktopts for the child and immediately consumes the
> request's reference. This can free the skb while the retransmit path is
> still reading its IPv6 header, causing a use-after-free.
> 
> Request migration has the same ownership problem. inet_reqsk_clone()
> copies pktopts without taking an skb reference, and reqsk_migrate_reset()
> then clears the old request's pointer. A CPU holding the old request can
> still be sending a SYNACK when the cloned request releases the only skb
> reference.
> 
> Do not consume pktopts while creating the child. Give each cloned request
> its own skb reference and let every request release its reference from
> tcp_v6_reqsk_destructor().
> 
> I found this while testing IPv6 TCP request-socket lifetime handling. The
> reproducer required no source changes, artificial delay, or fault
> injection beyond enabling KASAN. It triggered the use-after-free before
> this change. With this change, a 2000-round run completed without a KASAN
> report. A separate 10-round test recorded four successful request
> migrations and no KASAN report. Networking selftests were not run because
> none directly exercises this race.
> 
> Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
> Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yingjie Wang <x-professor@outlook.com>
> ---
>  net/ipv4/inet_connection_sock.c | 6 +++++-
>  net/ipv6/tcp_ipv6.c             | 7 ++++---
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 93b306d91b6d..66f8ce25aa38 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -936,6 +936,11 @@ static struct request_sock *inet_reqsk_clone(struct request_sock *req,
>  		      req->rsk_ops->obj_size - offsetof(struct sock, sk_dontcopy_end),
>  		      /* alloc is larger than struct, see above */);
>  
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (inet_rsk(nreq)->pktopts)
> +		skb_get(inet_rsk(nreq)->pktopts);
> +#endif
> +
>  	sk_node_init(&nreq_sk->sk_node);
>  	nreq_sk->sk_tx_queue_mapping = req_sk->sk_tx_queue_mapping;
>  #ifdef CONFIG_SOCK_RX_QUEUE_MAPPING
> @@ -982,7 +987,6 @@ static void reqsk_migrate_reset(struct request_sock *req)
>  	req->saved_syn = NULL;
>  #if IS_ENABLED(CONFIG_IPV6)
>  	inet_rsk(req)->ipv6_opt = NULL;
> -	inet_rsk(req)->pktopts = NULL;
>  #else
>  	inet_rsk(req)->ireq_opt = NULL;
>  #endif
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index e006a30360ea..0b51cb4ad960 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1522,11 +1522,12 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
>  	if (*own_req) {
>  		tcp_move_syn(newtp, req);
>  
> -		/* Clone pktoptions received with SYN, if we own the req */
> +		/* Clone pktoptions received with SYN. Keep the request's
> +		 * reference until the request is destroyed, as a concurrent
> +		 * SYN retransmit may still read it.
> +		 */
>  		if (ireq->pktopts) {
>  			newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
> -			consume_skb(ireq->pktopts);
> -			ireq->pktopts = NULL;

This is reported from another person in security@ mailing list
yesterday and Eric has a better fix.

pw-bot: rejected