[PATCH net] tcp: fix request_sock use-after-free during Fast Open accept()

[PATCH net] tcp: fix request_sock use-after-free during Fast Open accept()

[Hyunwoo Kim]

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)) {
 		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

Re: [PATCH net] tcp: fix request_sock use-after-free during Fast Open accept()

[Kuniyuki Iwashima]

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 ?


>  		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
> 

Re: [PATCH net] tcp: fix request_sock use-after-free during Fast Open accept()

[Hyunwoo Kim]

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
> > 

Re: [PATCH net] tcp: fix request_sock use-after-free during Fast Open accept()

[Kuniyuki Iwashima]

On Tue, Nov 10, 2026 at 9:43 PM Hyunwoo Kim <imv4bel@gmail.com> wrote:
>
> 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.

1-bit cannot be torn, and I guess KCSAN will not complain
even if there were a concurrent writer.


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

I feel it's misleading as data_race() implies the concurrent
writer, but when syn_fastopen_child is set, the bit itself
and the other bit fields are not written concurrently either.


>
> >
> >
> > >             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
> > >

Re: [PATCH net] tcp: fix request_sock use-after-free during Fast Open accept()

[Hyunwoo Kim]

On Tue, Nov 10, 2026 at 09:54:57PM -0800, Kuniyuki Iwashima wrote:
> On Tue, Nov 10, 2026 at 9:43 PM Hyunwoo Kim <imv4bel@gmail.com> wrote:
> >
> > 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.
> 
> 1-bit cannot be torn, and I guess KCSAN will not complain
> even if there were a concurrent writer.

Ah-ha.

> 
> 
> >
> > Still.. I agree that it looks a little odd. If you think it is
> > unnecessary, I will send a v2 without it.
> 
> I feel it's misleading as data_race() implies the concurrent
> writer, but when syn_fastopen_child is set, the bit itself
> and the other bit fields are not written concurrently either.

I'll send a cleaned-up v2.