mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Hyunwoo Kim <imv4bel@gmail.com>
To: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, ncardwell@google.com, dsahern@kernel.org,
	idosch@nvidia.com, kuniyu@google.com, horms@kernel.org,
	willemb@google.com, andrew+netdev@lunn.ch, kees@kernel.org,
	jiayuan.chen@linux.dev
Cc: kerneljasonxing@gmail.com, ij@kernel.org, martin.lau@kernel.org,
	shakeel.butt@linux.dev, matttbe@kernel.org, martineau@kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, imv4bel@gmail.com
Subject: Re: [PATCH net v2 6/8] tcp: fix use-after-free in the lockless listener path
Date: Tue, 1 Sep 2026 18:04:45 +0900	[thread overview]
Message-ID: <apaVLS6Srsa7SQhJ@v4bel> (raw)
In-Reply-To: <apaAskNL-IvdcR7h@v4bel>

On Tue, Sep 01, 2026 at 04:37:22PM +0900, Hyunwoo Kim wrote:
> On Mon, Aug 24, 2026 at 12:32:50PM +0900, Hyunwoo Kim wrote:
> > 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 <imv4bel@gmail.com>
> 
> 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().

...and tcp_abort() needs a fix too.

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b4237d0e994d6f..ce8e76052fe376 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -5139,6 +5139,9 @@ int tcp_abort(struct sock *sk, int err)

        if (sk->sk_state == TCP_LISTEN) {
                tcp_set_state(sk, TCP_CLOSE);
+               /* TCP BPF iterators run with RCU read-side protection. */
+               if (!has_current_bpf_ctx())
+                       synchronize_net();
                inet_csk_listen_stop(sk);
        }

> 
> 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.
> 
> 
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index b4237d0e994d6f..ae6ab3b22beb53 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3373,6 +3373,8 @@ int tcp_disconnect(struct sock *sk, int flags)
> 
>  	/* ABORT function of RFC793 */
>  	if (old_state == TCP_LISTEN) {
> +		/* Wait for lockless listener receive paths to finish. */
> +		synchronize_net();
>  		inet_csk_listen_stop(sk);
>  	} else if (unlikely(tp->repair)) {
>  		WRITE_ONCE(sk->sk_err, ECONNABORTED);
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index b474d03620a75d..e41a5e1bf8dbf3 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3432,7 +3432,7 @@ static __poll_t mptcp_check_readable(struct sock *sk)
>  	return mptcp_epollin_ready(sk) ? EPOLLIN | EPOLLRDNORM : 0;
>  }
> 
> -static void mptcp_check_listen_stop(struct sock *sk)
> +static void mptcp_check_listen_stop(struct sock *sk, bool sync_net)
>  {
>  	struct sock *ssk;
> 
> @@ -3446,6 +3446,9 @@ static void mptcp_check_listen_stop(struct sock *sk)
> 
>  	lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
>  	tcp_set_state(ssk, TCP_CLOSE);
> +	if (sync_net)
> +		/* Wait for lockless listener receive paths to finish. */
> +		synchronize_net();
>  	mptcp_subflow_queue_clean(sk, ssk);
>  	inet_csk_listen_stop(ssk);
>  	mptcp_event_pm_listener(ssk, MPTCP_EVENT_LISTENER_CLOSED);
> @@ -3462,7 +3465,7 @@ bool __mptcp_close(struct sock *sk, long timeout)
>  	WRITE_ONCE(sk->sk_shutdown, SHUTDOWN_MASK);
> 
>  	if ((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) {
> -		mptcp_check_listen_stop(sk);
> +		mptcp_check_listen_stop(sk, false);
>  		mptcp_set_state(sk, TCP_CLOSE);
>  		goto cleanup;
>  	}
> @@ -3593,7 +3596,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
>  	if (msk->fastopening)
>  		return -EBUSY;
> 
> -	mptcp_check_listen_stop(sk);
> +	mptcp_check_listen_stop(sk, true);
>  	mptcp_set_state(sk, TCP_CLOSE);
> 
>  	mptcp_stop_rtx_timer(sk);
> 
> 
> Best regards,
> Hyunwoo Kim

  parent reply	other threads:[~2026-09-01  9:04 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-24  3:32 [PATCH net v2 0/8] net: fixes for requests completing on a socket that no longer listens Hyunwoo Kim
2026-08-24  3:32 ` [PATCH net v2 1/8] tcp: fix use-after-free of the listener's ipv6_pinfo after IPV6_ADDRFORM Hyunwoo Kim
2026-08-24  3:32 ` [PATCH net v2 2/8] tcp: fix imbalanced icsk_accept_queue count in tcp_check_req() Hyunwoo Kim
2026-08-24  3:32 ` [PATCH net v2 3/8] ipv6: fix request socket use-after-free after IPV6_ADDRFORM Hyunwoo Kim
2026-08-24  3:32 ` [PATCH net v2 4/8] net: fix out-of-bounds write in sk_clone() racing with IPV6_ADDRFORM Hyunwoo Kim
2026-08-24  3:32 ` [PATCH net v2 5/8] tcp: do not inherit out_of_order_queue from parent Hyunwoo Kim
2026-08-24  3:32 ` [PATCH net v2 6/8] tcp: fix use-after-free in the lockless listener path Hyunwoo Kim
2026-09-01  7:37   ` Hyunwoo Kim
2026-09-01  8:03     ` Paolo Abeni
2026-09-01  9:06       ` Hyunwoo Kim
2026-09-01 15:51       ` Jakub Kicinski
2026-09-01 15:57         ` Eric Dumazet
2026-09-03 19:16           ` Hyunwoo Kim
2026-09-03 19:28             ` Kuniyuki Iwashima
2026-09-03 19:31               ` Hyunwoo Kim
2026-09-01  9:04     ` Hyunwoo Kim [this message]
2026-08-24  3:32 ` [PATCH net v2 7/8] net: clear sk_tsq_flags in sk_clone() Hyunwoo Kim
2026-08-24  3:32 ` [PATCH net v2 8/8] tcp: do not inherit retransmit state from parent Hyunwoo Kim
2026-08-24  8:30 ` [PATCH net v2 0/8] net: fixes for requests completing on a socket that no longer listens David Laight
2026-08-24 12:56   ` Hyunwoo Kim
2026-08-31 23:35 ` Jakub Kicinski
2026-09-01  7:40   ` Hyunwoo Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=apaVLS6Srsa7SQhJ@v4bel \
    --to=imv4bel@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=ij@kernel.org \
    --cc=jiayuan.chen@linux.dev \
    --cc=kees@kernel.org \
    --cc=kerneljasonxing@gmail.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@kernel.org \
    --cc=martineau@kernel.org \
    --cc=matttbe@kernel.org \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shakeel.butt@linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®