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

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

[Yingjie Wang]

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;
 			if (newnp->pktoptions)
 				tcp_v6_restore_cb(newnp->pktoptions);
 		}

base-commit: cb284914e749388eef402938bdb3115fe1f6d337
-- 
2.43.0

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

[Kuniyuki Iwashima]

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

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

[Eric Dumazet]

Le lun. 2 nov. 2026 à 08:03, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> 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

Indeed, I will add another 'Reported-by: Yingjie Wang
<x-professor@outlook.com>' tag.

Thanks.