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
> >
next prev parent 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox