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

[PATCH net v2] 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) on a 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 more likely outcome of this race, even without a subsequent
listen(), is a leak. If reqsk_fastopen_remove() inserts a request_sock
after inet_csk_listen_stop() observes an empty list and returns, the
request_sock is stranded in rskq_rst_head and keeps a reference on the
listener. The socket is already in TCP_CLOSE, so a subsequent close()
does not call inet_csk_listen_stop() again.

If listen() is called again on the same socket before the delayed list
insertion, the insertion can overlap the queue initialization in
reqsk_queue_alloc(). If it occurs after rskq_rst_head is set to NULL but
before rskq_rst_tail is set to NULL, 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. With
this synchronization, reqsk_fastopen_remove() either inserts the
request_sock before inet_csk_listen_stop() acquires the lock, in which
case inet_csk_listen_stop() reclaims it, or sees that the listener is
not in TCP_LISTEN and does not insert anything.

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>
---
Changes in v2:
- Drop the reqsk_queue_alloc() locking hunk and keep only the
  inet_csk_listen_stop() change. (Eric)
- Document the more likely request_sock leak and the listener reference
  it holds. (Eric)
- v1: https://lore.kernel.org/all/avGpsTb2hsGIjSRk@v4bel/
---
 net/ipv4/inet_connection_sock.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index e56beec438773..b9f3efb671017 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -1533,17 +1533,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 v2] tcp: fix NULL pointer dereference when restarting a Fast Open listener

[Kuniyuki Iwashima]

On Tue, 10 Nov 2026 23:04:44 +0900, Hyunwoo Kim wrote:
> 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) on a 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 more likely outcome of this race, even without a subsequent
> listen(), is a leak. If reqsk_fastopen_remove() inserts a request_sock
> after inet_csk_listen_stop() observes an empty list and returns, the
> request_sock is stranded in rskq_rst_head and keeps a reference on the
> listener. The socket is already in TCP_CLOSE, so a subsequent close()
> does not call inet_csk_listen_stop() again.
> 
> If listen() is called again on the same socket before the delayed list
> insertion, the insertion can overlap the queue initialization in
> reqsk_queue_alloc(). If it occurs after rskq_rst_head is set to NULL but
> before rskq_rst_tail is set to NULL, 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. With
> this synchronization, reqsk_fastopen_remove() either inserts the
> request_sock before inet_csk_listen_stop() acquires the lock, in which
> case inet_csk_listen_stop() reclaims it, or sees that the listener is
> not in TCP_LISTEN and does not insert anything.
> 
> 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>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch is meant to stop request_socks from being stranded in
  rskq_rst_head while they hold a listener reference, but another path…

--

tcp: fix NULL pointer dereference when restarting a Fast Open listener

This patch makes inet_csk_listen_stop() always take fastopenq.lock and
detach rskq_rst_head while holding it. That serializes the drain against
reqsk_fastopen_remove() adding entries to the RST list. The goal is to
stop request_socks from being stranded on rskq_rst_head while holding a
listener reference, and to avoid the NULL rskq_rst_tail dereference seen
when listen() is called again.

> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 37cc8606d6835..9ca59dca5d184 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -1533,17 +1533,15 @@ void inet_csk_listen_stop(struct sock *sk)

[ ... ]

> +	/* 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);
>  }

[Severity: High]
Is this fix complete? The commit message says:

    With this synchronization, reqsk_fastopen_remove() either inserts the
    request_sock before inet_csk_listen_stop() acquires the lock, in which
    case inet_csk_listen_stop() reclaims it, or sees that the listener is
    not in TCP_LISTEN and does not insert anything.

That argument only holds if every exit from TCP_LISTEN goes through
inet_csk_listen_stop(). The failure path in inet_csk_listen_start()
does not:

net/ipv4/inet_connection_sock.c:inet_csk_listen_start() {
    ...
	reqsk_queue_alloc(&icsk->icsk_accept_queue);
    ...
	inet_sk_state_store(sk, TCP_LISTEN);
	err = sk->sk_prot->get_port(sk, inet->inet_num);
	if (!err) {
		...
		err = sk->sk_prot->hash(sk);

		if (likely(!err))
			return 0;
	}

	inet_sk_set_state(sk, TCP_CLOSE);
	return err;
}

Take a listener that went through shutdown(SHUT_RD) or connect(AF_UNSPEC)
while an accepted Fast Open child was still in SYN_RECV. Then listen() is
called on it again. If an RST for that child arrives after TCP_LISTEN is
stored, reqsk_fastopen_remove() passes this check under fastopenq->lock:

	if (!reset || lsk->sk_state != TCP_LISTEN) {

It then links req onto rskq_rst_head/rskq_rst_tail without calling
reqsk_put().

Now suppose get_port() fails afterwards, for example with EADDRINUSE
because another SO_REUSEADDR socket started listening on the port, or
hash() fails. The socket goes back to TCP_CLOSE and rskq_rst_head is never
drained.

A later close() will not reclaim the list, because __tcp_close() only
calls inet_csk_listen_stop() when sk_state == TCP_LISTEN. A later
successful listen() just overwrites the list in reqsk_queue_alloc():

	queue->fastopenq.rskq_rst_head = NULL;
	queue->fastopenq.rskq_rst_tail = NULL;

Does this leak the request_sock? If so, it also leaks the listener
reference taken in reqsk_alloc_noprof(), which only
sock_put(req->rsk_listener) in __reqsk_free() drops. That would pin the
listener sock and its netns reference too.

This looks like the same leak the commit message describes, reached
through a different TCP_LISTEN -> TCP_CLOSE transition.

Should the inet_csk_listen_start() error path also drain fastopenq under
fastopenq.lock? Alternatively, could the drain move to a place that covers
both paths?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/avMkfJrXhCFXn8M_%40v4bel