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

[Eric Dumazet]

Le lun. 2 nov. 2026 à 08:03, Kuniyuki Iwashima <kuniyu@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> > Signed-off-by: Yingjie Wang <x-professor@xxxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxxx>' tag.

Thanks.