[PATCH net] tcp: fix NULL pointer dereference when restarting a Fast Open listener

[PATCH net] tcp: fix NULL pointer dereference when restarting a Fast Open listener

[Hyunwoo Kim]

When a valid RST is received for a Fast Open child accepted before the
three-way handshake completes, reqsk_fastopen_remove() adds the
request_sock to the listener's RST list if the listener is in TCP_LISTEN.

An application can call shutdown(SHUT_RD) and then listen() on the same
listener socket. inet_csk_listen_stop() checks rskq_rst_head without
holding fastopenq.lock. If rskq_rst_head is NULL, it does not acquire the
lock and therefore does not synchronize with an insertion by
reqsk_fastopen_remove() after that function has passed the TCP_LISTEN
check.

The subsequent reqsk_queue_alloc() also initializes rskq_rst_head,
rskq_rst_tail, and qlen in that order without holding the lock. If a
request_sock is added between the initialization of rskq_rst_head and
rskq_rst_tail, rskq_rst_head points to the request_sock while
rskq_rst_tail is NULL.

When a valid RST is later received for another accepted Fast Open child,
rskq_rst_head is not NULL, so reqsk_fastopen_remove() attempts to write
the request_sock pointer to rskq_rst_tail->dl_next while rskq_rst_tail is
NULL. KASAN reports:

  BUG: KASAN: null-ptr-deref in reqsk_fastopen_remove+0x147/0x210
  Write of size 8 at addr 0000000000000088 by task swapper/3/0
  ...
  Call Trace:
   <IRQ>
   ...
   reqsk_fastopen_remove+0x147/0x210
   tcp_check_req+0xa70/0x1060
   ...
   tcp_rcv_state_process+0x1cb/0x2ae0
   ...
   tcp_v4_do_rcv+0x1fe/0x740
   tcp_v4_rcv+0x1744/0x2070
   ...
   ip_protocol_deliver_rcu+0x5c/0x280
   ip_local_deliver_finish+0x171/0x330
   ip_local_deliver+0x107/0x370
   ...
   </IRQ>

To widen the race window, the issue was reproduced after adding
mdelay(3000) immediately after reqsk_fastopen_remove() checks that the
listener is in TCP_LISTEN and immediately after reqsk_queue_alloc() sets
rskq_rst_head to NULL.

Make inet_csk_listen_stop() acquire fastopenq.lock regardless of the
value of rskq_rst_head and detach the list while holding the lock. If
reqsk_fastopen_remove() has already passed the TCP_LISTEN check,
inet_csk_listen_stop() waits until the list insertion completes.

Also initialize rskq_rst_head, rskq_rst_tail, and qlen under the same
lock in reqsk_queue_alloc(). Drop the references to the detached
request_sock objects after releasing the lock.

Fixes: 0536fcc039a8 ("tcp: prepare fastopen code for upcoming listener changes")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 net/ipv4/inet_connection_sock.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index e56beec438773..5590fc7efb88d 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -1333,9 +1333,11 @@ static int inet_ulp_can_listen(const struct sock *sk)
 
 static void reqsk_queue_alloc(struct request_sock_queue *queue)
 {
+	spin_lock_bh(&queue->fastopenq.lock);
 	queue->fastopenq.rskq_rst_head = NULL;
 	queue->fastopenq.rskq_rst_tail = NULL;
 	queue->fastopenq.qlen = 0;
+	spin_unlock_bh(&queue->fastopenq.lock);
 
 	queue->rskq_accept_head = NULL;
 }
@@ -1533,17 +1535,15 @@ void inet_csk_listen_stop(struct sock *sk)
 		if (!has_current_bpf_ctx())
 			cond_resched();
 	}
-	if (queue->fastopenq.rskq_rst_head) {
-		/* Free all the reqs queued in rskq_rst_head. */
-		spin_lock_bh(&queue->fastopenq.lock);
-		req = queue->fastopenq.rskq_rst_head;
-		queue->fastopenq.rskq_rst_head = NULL;
-		spin_unlock_bh(&queue->fastopenq.lock);
-		while (req != NULL) {
-			next = req->dl_next;
-			reqsk_put(req);
-			req = next;
-		}
+	/* Free all the reqs queued in rskq_rst_head. */
+	spin_lock_bh(&queue->fastopenq.lock);
+	req = queue->fastopenq.rskq_rst_head;
+	queue->fastopenq.rskq_rst_head = NULL;
+	spin_unlock_bh(&queue->fastopenq.lock);
+	while (req != NULL) {
+		next = req->dl_next;
+		reqsk_put(req);
+		req = next;
 	}
 	WARN_ON_ONCE(sk->sk_ack_backlog);
 }
-- 
2.43.0

Re: [PATCH net] tcp: fix NULL pointer dereference when restarting a Fast Open listener

[Eric Dumazet]

Le lun. 9 nov. 2026 à 12:08, Hyunwoo Kim <imv4bel@gmail.com> a écrit :
>
> When a valid RST is received for a Fast Open child accepted before the
> three-way handshake completes, reqsk_fastopen_remove() adds the
> request_sock to the listener's RST list if the listener is in TCP_LISTEN.
>
> An application can call shutdown(SHUT_RD) and then listen() on the same
> listener socket. inet_csk_listen_stop() checks rskq_rst_head without
> holding fastopenq.lock. If rskq_rst_head is NULL, it does not acquire the
> lock and therefore does not synchronize with an insertion by
> reqsk_fastopen_remove() after that function has passed the TCP_LISTEN
> check.
>
> The subsequent reqsk_queue_alloc() also initializes rskq_rst_head,
> rskq_rst_tail, and qlen in that order without holding the lock. If a
> request_sock is added between the initialization of rskq_rst_head and
> rskq_rst_tail, rskq_rst_head points to the request_sock while
> rskq_rst_tail is NULL.
>
> When a valid RST is later received for another accepted Fast Open child,
> rskq_rst_head is not NULL, so reqsk_fastopen_remove() attempts to write
> the request_sock pointer to rskq_rst_tail->dl_next while rskq_rst_tail is
> NULL. KASAN reports:
>
>   BUG: KASAN: null-ptr-deref in reqsk_fastopen_remove+0x147/0x210
>   Write of size 8 at addr 0000000000000088 by task swapper/3/0
>   ...
>   Call Trace:
>    <IRQ>
>    ...
>    reqsk_fastopen_remove+0x147/0x210
>    tcp_check_req+0xa70/0x1060
>    ...
>    tcp_rcv_state_process+0x1cb/0x2ae0
>    ...
>    tcp_v4_do_rcv+0x1fe/0x740
>    tcp_v4_rcv+0x1744/0x2070
>    ...
>    ip_protocol_deliver_rcu+0x5c/0x280
>    ip_local_deliver_finish+0x171/0x330
>    ip_local_deliver+0x107/0x370
>    ...
>    </IRQ>
>
> To widen the race window, the issue was reproduced after adding
> mdelay(3000) immediately after reqsk_fastopen_remove() checks that the
> listener is in TCP_LISTEN and immediately after reqsk_queue_alloc() sets
> rskq_rst_head to NULL.
>
> Make inet_csk_listen_stop() acquire fastopenq.lock regardless of the
> value of rskq_rst_head and detach the list while holding the lock. If
> reqsk_fastopen_remove() has already passed the TCP_LISTEN check,
> inet_csk_listen_stop() waits until the list insertion completes.
>
> Also initialize rskq_rst_head, rskq_rst_tail, and qlen under the same
> lock in reqsk_queue_alloc(). Drop the references to the detached
> request_sock objects after releasing the lock.
>
> Fixes: 0536fcc039a8 ("tcp: prepare fastopen code for upcoming listener changes")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  net/ipv4/inet_connection_sock.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index e56beec438773..5590fc7efb88d 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -1333,9 +1333,11 @@ static int inet_ulp_can_listen(const struct sock *sk)
>
>  static void reqsk_queue_alloc(struct request_sock_queue *queue)
>  {
> +       spin_lock_bh(&queue->fastopenq.lock);
>         queue->fastopenq.rskq_rst_head = NULL;
>         queue->fastopenq.rskq_rst_tail = NULL;
>         queue->fastopenq.qlen = 0;
> +       spin_unlock_bh(&queue->fastopenq.lock);

I do not think this spin_lock_bh() is _needed_ here, this hunk is
misleading/distracting.

Once inet_csk_listen_stop() takes fastopenq.lock unconditionally,
reqsk_fastopen_remove() either inserted its req before we grabbed the lock
(and we free it), or it will see sk_state != TCP_LISTEN and will not insert
anything. Nothing can be added to rskq_rst_head until inet_csk_listen_start()
sets TCP_LISTEN again, after reqsk_queue_alloc().

The double mdelay() needed to reproduce the NULL deref shows the window:
shutdown() + listen() would have to complete while reqsk_fastopen_remove()
is between its TCP_LISTEN check and the list insertion.

Please let's focus on the real issue, which is solely in
inet_csk_listen_stop().

Also note the more likely outcome of this race is a leak: a req stranded
in rskq_rst_head holds a reference on the listener, and a subsequent
close() will not call inet_csk_listen_stop() again. This is worth
mentioning in the changelog.

Please send a v2 with only the inet_csk_listen_stop() part.

Thanks.

pw-bo: cr

Re: [PATCH net] tcp: fix NULL pointer dereference when restarting a Fast Open listener

[Hyunwoo Kim]

On Mon, Nov 09, 2026 at 01:55:44PM +0100, Eric Dumazet wrote:
> Le lun. 9 nov. 2026 à 12:08, Hyunwoo Kim <imv4bel@gmail.com> a écrit :
> >
> > When a valid RST is received for a Fast Open child accepted before the
> > three-way handshake completes, reqsk_fastopen_remove() adds the
> > request_sock to the listener's RST list if the listener is in TCP_LISTEN.
> >
> > An application can call shutdown(SHUT_RD) and then listen() on the same
> > listener socket. inet_csk_listen_stop() checks rskq_rst_head without
> > holding fastopenq.lock. If rskq_rst_head is NULL, it does not acquire the
> > lock and therefore does not synchronize with an insertion by
> > reqsk_fastopen_remove() after that function has passed the TCP_LISTEN
> > check.
> >
> > The subsequent reqsk_queue_alloc() also initializes rskq_rst_head,
> > rskq_rst_tail, and qlen in that order without holding the lock. If a
> > request_sock is added between the initialization of rskq_rst_head and
> > rskq_rst_tail, rskq_rst_head points to the request_sock while
> > rskq_rst_tail is NULL.
> >
> > When a valid RST is later received for another accepted Fast Open child,
> > rskq_rst_head is not NULL, so reqsk_fastopen_remove() attempts to write
> > the request_sock pointer to rskq_rst_tail->dl_next while rskq_rst_tail is
> > NULL. KASAN reports:
> >
> >   BUG: KASAN: null-ptr-deref in reqsk_fastopen_remove+0x147/0x210
> >   Write of size 8 at addr 0000000000000088 by task swapper/3/0
> >   ...
> >   Call Trace:
> >    <IRQ>
> >    ...
> >    reqsk_fastopen_remove+0x147/0x210
> >    tcp_check_req+0xa70/0x1060
> >    ...
> >    tcp_rcv_state_process+0x1cb/0x2ae0
> >    ...
> >    tcp_v4_do_rcv+0x1fe/0x740
> >    tcp_v4_rcv+0x1744/0x2070
> >    ...
> >    ip_protocol_deliver_rcu+0x5c/0x280
> >    ip_local_deliver_finish+0x171/0x330
> >    ip_local_deliver+0x107/0x370
> >    ...
> >    </IRQ>
> >
> > To widen the race window, the issue was reproduced after adding
> > mdelay(3000) immediately after reqsk_fastopen_remove() checks that the
> > listener is in TCP_LISTEN and immediately after reqsk_queue_alloc() sets
> > rskq_rst_head to NULL.
> >
> > Make inet_csk_listen_stop() acquire fastopenq.lock regardless of the
> > value of rskq_rst_head and detach the list while holding the lock. If
> > reqsk_fastopen_remove() has already passed the TCP_LISTEN check,
> > inet_csk_listen_stop() waits until the list insertion completes.
> >
> > Also initialize rskq_rst_head, rskq_rst_tail, and qlen under the same
> > lock in reqsk_queue_alloc(). Drop the references to the detached
> > request_sock objects after releasing the lock.
> >
> > Fixes: 0536fcc039a8 ("tcp: prepare fastopen code for upcoming listener changes")
> > Cc: stable@vger.kernel.org
> > Assisted-by: LLM
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > ---
> >  net/ipv4/inet_connection_sock.c | 22 +++++++++++-----------
> >  1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> > index e56beec438773..5590fc7efb88d 100644
> > --- a/net/ipv4/inet_connection_sock.c
> > +++ b/net/ipv4/inet_connection_sock.c
> > @@ -1333,9 +1333,11 @@ static int inet_ulp_can_listen(const struct sock *sk)
> >
> >  static void reqsk_queue_alloc(struct request_sock_queue *queue)
> >  {
> > +       spin_lock_bh(&queue->fastopenq.lock);
> >         queue->fastopenq.rskq_rst_head = NULL;
> >         queue->fastopenq.rskq_rst_tail = NULL;
> >         queue->fastopenq.qlen = 0;
> > +       spin_unlock_bh(&queue->fastopenq.lock);
> 
> I do not think this spin_lock_bh() is _needed_ here, this hunk is
> misleading/distracting.
> 
> Once inet_csk_listen_stop() takes fastopenq.lock unconditionally,
> reqsk_fastopen_remove() either inserted its req before we grabbed the lock
> (and we free it), or it will see sk_state != TCP_LISTEN and will not insert
> anything. Nothing can be added to rskq_rst_head until inet_csk_listen_start()
> sets TCP_LISTEN again, after reqsk_queue_alloc().

Ah, yes, that spin lock is unnecessary.

> 
> The double mdelay() needed to reproduce the NULL deref shows the window:
> shutdown() + listen() would have to complete while reqsk_fastopen_remove()
> is between its TCP_LISTEN check and the list insertion.
> 
> Please let's focus on the real issue, which is solely in
> inet_csk_listen_stop().
> 
> Also note the more likely outcome of this race is a leak: a req stranded
> in rskq_rst_head holds a reference on the listener, and a subsequent
> close() will not call inet_csk_listen_stop() again. This is worth
> mentioning in the changelog.
> 
> Please send a v2 with only the inet_csk_listen_stop() part.

Ok. I'll do that.


Best regards,
Hyunwoo Kim