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