Re: [PATCH net v2 6/8] tcp: fix use-after-free in the lockless listener path

On Tue, Sep 01, 2026 at 10:03:51AM +0200, Paolo Abeni wrote:

On 9/1/26 9:37 AM, Hyunwoo Kim wrote:

quoted

On Mon, Aug 24, 2026 at 12:32:50PM +0900, Hyunwoo Kim wrote:

quoted

tcp_v{4,6}_rcv() calls tcp_v{4,6}_do_rcv() without holding the socket
lock when sk->sk_state is TCP_LISTEN. Every other path into
tcp_v{4,6}_do_rcv() holds it.

tcp_v{4,6}_do_rcv() and tcp_rcv_state_process() below it read
sk->sk_state again. A listener can leave TCP_LISTEN through
connect(AF_UNSPEC), and if that happens in between, the second read
returns a different state.

tcp_rcv_established() or tcp_rcv_state_process() then runs without the
lock. If the second read returns TCP_SYN_SENT, the incoming SYN is
treated as a crossed SYN and reaches tcp_send_synack(). When the SYN skb
at the head of the retransmit queue is skb_cloned(), that function
replaces it with a copy and releases the original with
tcp_rtx_queue_unlink_and_free().

The original is the skb that a thread on another CPU is transmitting
right now in __tcp_transmit_skb(). skb_cloned() is true because the
clone made for that transmit is still alive. Once the transmit returns,
tcp_update_skb_after_send() calls list_move_tail() on the skb's
tcp_tsorted_anchor.

In short:

  socket(AF_INET) -> bind() -> listen()   // the socket that changes state
  socket(AF_INET) -> bind() -> listen()   // the peer

  Several threads keep opening new sockets and connecting to the first
  socket's address.

  Another thread repeats this on the first socket:
    connect(AF_UNSPEC)      // TCP_LISTEN -> TCP_CLOSE
    connect(peer address)   // TCP_CLOSE -> TCP_SYN_SENT
                            // another CPU still sees a listener, handles
                            // one of those SYNs without the lock and
                            // releases the SYN skb that this connect()
                            // is transmitting
                            // -> use-after-free
    connect(AF_UNSPEC)
    listen()                // TCP_LISTEN again

KASAN log:

  BUG: KASAN: slab-use-after-free in __list_del_entry_valid_or_report+0x14/0x140
  Read of size 8 at addr ffff88800a5d1460 by task poc/125
  ...
  Call Trace:
   __list_del_entry_valid_or_report+0x14/0x140
   tcp_update_skb_after_send+0x62/0x170
   __tcp_transmit_skb+0xe33/0x1e40
   tcp_connect+0x1b67/0x2490
   tcp_v4_connect+0x998/0xab0
   __inet_stream_connect+0x22c/0x700
   inet_stream_connect+0x48/0x70
   __sys_connect+0x101/0x130
  ...
  Allocated by task 125:
   __alloc_skb+0xd1/0x370
   tcp_stream_alloc_skb+0x2d/0x2b0
   tcp_connect+0x72d/0x2490
   tcp_v4_connect+0x998/0xab0
   __inet_stream_connect+0x22c/0x700
   inet_stream_connect+0x48/0x70
   __sys_connect+0x101/0x130
  ...
  The buggy address belongs to the object at ffff88800a5d1400
   which belongs to the cache skbuff_fclone_cache of size 472

Instead of taking the lock, keep the lockless path from reading
sk->sk_state again to decide how to process the packet. Move the
TCP_LISTEN handling out of tcp_rcv_state_process() into
tcp_rcv_listen_state_process(), and let the TCP_LISTEN branch of
tcp_v{4,6}_rcv() call a new tcp_v{4,6}_rcv_listen(). Listener processing
does not change. The TCP_LISTEN arm of tcp_v{4,6}_do_rcv() is left
alone, because a socket can finish listen() after the state check and a
backlogged skb is then processed there.

Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <redacted>

Looking at this further, unhashing the listener and then calling
synchronize_net() lets the disconnect path handle it. MPTCP needs a fix
too, though, because it closes and reuses the first subflow directly
without going through tcp_disconnect().

This looks like a more palatable approach: this patch in the current
format looked way too invasive to me.

quoted

This also closes the trigger path for patches 4, 5 and 8. I would still
keep those, since they add no work to the fast path and they remove the
root cause itself. Their changelogs would have to change though.

I'm unsure accepting new connections is not fast-path: the connection
per second rate is a relevant metric for a sever, even if the additional
cleanup is possibly not visible in most benchmarks.

Still I would avoid additional unneeded patches.

Yeah, I will drop those patches when I send v3.


Best regards,
Hyunwoo Kim