All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hyunwoo Kim <imv4bel@gmail.com>
To: Kuniyuki Iwashima <kuniyu@google.com>
Cc: Eric Dumazet <edumazet@kernel.org>,
	Neal Cardwell <ncardwell@google.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	netdev@vger.kernel.org, imv4bel@gmail.com
Subject: Re: [PATCH net] tcp: fix request_sock use-after-free during Fast Open accept()
Date: Wed, 11 Nov 2026 14:43:11 +0900	[thread overview]
Message-ID: <avQAb-Gb7FRYNNgO@v4bel> (raw)
In-Reply-To: <avP5PfDz6WfnvckT@google.com>

On Wed, Nov 11, 2026 at 05:13:15AM +0000, Kuniyuki Iwashima wrote:
> On Mon, 09 Nov 2026 15:29:10 +0900, Hyunwoo Kim wrote:
> > Commit e3d95ad7da4f ("inet: avoid fastopen lock for regular accept()")
> > made inet_csk_accept() check tfo_listener before taking fastopenq.lock
> > to avoid taking the lock for connections that did not use Fast Open.
> > 
> > reqsk_fastopen_remove() clears tfo_listener while holding fastopenq.lock,
> > then reads req->sk. If inet_csk_accept() concurrently observes
> > tfo_listener after it has been cleared, it skips fastopenq.lock and
> > drops the last reference to the request_sock. This can free the
> > request_sock while reqsk_fastopen_remove() is still using it.
> > 
> > If the freed memory is reused for a new request_sock with req->sk ==
> > NULL, the reset path in reqsk_fastopen_remove() can append the new
> > request_sock to the original listener's list of requests that caused an
> > RST. After the new request_sock is freed, the next append writes through
> > the stale rskq_rst_tail into freed memory.
> > 
> > KASAN reported the following use-after-free when writing
> > fastopenq->rskq_rst_tail->dl_next = req:
> > 
> >   BUG: KASAN: slab-use-after-free in reqsk_fastopen_remove+0x142/0x210
> >   Write of size 8 at addr ffff888108230688 by task swapper/5/0
> >   ...
> >   Call Trace:
> >    <IRQ>
> >    ...
> >    reqsk_fastopen_remove+0x142/0x210
> >    tcp_check_req+0x96f/0xdd0
> >    ...
> >    tcp_rcv_state_process+0x19c/0x26b0
> >    ...
> >    tcp_v4_do_rcv+0x16e/0x490
> >    tcp_v4_rcv+0x14a0/0x1af0
> >    ...
> >    ip_protocol_deliver_rcu+0x39/0x1e0
> >    ip_local_deliver_finish+0x124/0x1e0
> >    ip_local_deliver+0xfb/0x1e0
> >    ...
> >    </IRQ>
> > 
> > To widen the race windows, mdelay(5) was added after newsk = req->sk,
> > mdelay(20) after the if (req) reqsk_put(req) block, and mdelay(100)
> > after tcp_rsk(req)->tfo_listener = false. The KASAN report above was
> > reproduced with this kernel.
> > 
> > For the lockless check, use tcp_sk(newsk)->syn_fastopen_child instead of
> > tfo_listener, which reqsk_fastopen_remove() clears.
> > 
> > syn_fastopen_child is a field of the Fast Open child TCP socket
> > referenced by newsk, not of the request_sock. It is set before the child
> > socket is added to the accept queue. The paths that call
> > reqsk_fastopen_remove() in response to a remote ACK or RST do not clear
> > this field.
> > 
> > inet_csk_accept() uses this field to take fastopenq.lock only for Fast
> > Open child sockets and rechecks tfo_listener while holding the lock.
> > Connections that did not use Fast Open continue to avoid the lock.
> > 
> > Other bitfields sharing the same byte can be modified concurrently, so
> > annotate the lockless read of syn_fastopen_child with data_race().
> > 
> > Fixes: e3d95ad7da4f ("inet: avoid fastopen lock for regular accept()")
> > Cc: stable@vger.kernel.org
> > Assisted-by: LLM
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > ---
> >  net/ipv4/inet_connection_sock.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> > index e56beec438773..e5e7b844fed44 100644
> > --- a/net/ipv4/inet_connection_sock.c
> > +++ b/net/ipv4/inet_connection_sock.c
> > @@ -683,7 +683,8 @@ struct sock *inet_csk_accept(struct sock *sk, struct proto_accept_arg *arg)
> >  	newsk = req->sk;
> >  
> >  	if (sk->sk_protocol == IPPROTO_TCP &&
> > -	    tcp_rsk(req)->tfo_listener) {
> > +	    /* Stable while reqsk removal can race; adjacent bitfields may change. */
> > +	    data_race(!!tcp_sk(newsk)->syn_fastopen_child)) {
> 
> Are data_race() and !! necessary ?

It was intended to avoid spurious KCSAN reports.

Still.. I agree that it looks a little odd. If you think it is 
unnecessary, I will send a v2 without it.

> 
> 
> >  		spin_lock_bh(&queue->fastopenq.lock);
> >  		if (tcp_rsk(req)->tfo_listener) {
> >  			/* We are still waiting for the final ACK from 3WHS
> > -- 
> > 2.43.0
> > 

  reply	other threads:[~2026-11-11  5:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-09  6:29 [PATCH net] tcp: fix request_sock use-after-free during Fast Open accept() Hyunwoo Kim
2026-11-11  5:13 ` Kuniyuki Iwashima
2026-11-11  5:43   ` Hyunwoo Kim [this message]
2026-11-11  5:54     ` Kuniyuki Iwashima
2026-11-11  6:05       ` Hyunwoo Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=avQAb-Gb7FRYNNgO@v4bel \
    --to=imv4bel@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.