[PATCH net v2 1/8] tcp: fix use-after-free of the listener's ipv6_pinfo after IPV6_ADDRFORM

[PATCH net v2 1/8] tcp: fix use-after-free of the listener's ipv6_pinfo after IPV6_ADDRFORM

[Hyunwoo Kim]

IPV6_ADDRFORM switches an established AF_INET6 TCP socket to tcp_prot and
ipv4_specific. The socket is still a tcp6_sock, so ->pinet6 keeps pointing
at the ipv6_pinfo inside it, and sk_destruct is left alone because
commit d38afeec26ed ("tcp/udp: Call inet6_destroy_sock() in IPv6
sk->sk_destruct().") uses it to clean up the IPv6 resources.

After connect(AF_UNSPEC) the socket can listen() again. Its children are
then created by tcp_v4_syn_recv_sock() with opt_child_init NULL, and
sk_clone() allocates them from tcp_prot, so each one is a plain tcp_sock
that inherits ->pinet6 and sk_destruct from the listener.
tcp_v6_mapped_child_init(), added by commit 858d2a4f67ff ("tcp: fix
potential race in tcp_v6_syn_recv_sock()"), would overwrite ->pinet6, but
tcp_v6_syn_recv_sock() is the only caller that passes it and it is not
involved here.

A child can outlive the listener. INET_ECN_xmit() and INET_ECN_dontxmit()
test inet6_sk(sk) and not sk_family, so tcp_ecn_send() updates np->tclass
through the stale pointer, and the child's destructor runs
inet6_cleanup_sock() on the freed listener.

In short:

  socket(AF_INET6) -> bind -> listen
      // a client connects over IPv4
  accept()                              // the child is v4-mapped
  setsockopt(IPV6_ADDRFORM, PF_INET)    // it becomes an AF_INET socket
  connect(AF_UNSPEC) -> bind -> listen  // reuse it as an IPv4 server
      // a client connects again
  accept()
  close(the listener)
  close(the accepted socket)            // use-after-free

KASAN log:

  BUG: KASAN: slab-use-after-free in __tcp_transmit_skb+0x1070/0x2020
  Read of size 1 at addr ffff888016671af3 by task poc/111
  ...
  Call Trace:
   __tcp_transmit_skb+0x1070/0x2020
   tcp_write_xmit+0xace/0x3380
   __tcp_push_pending_frames+0x58/0x180
   __tcp_close+0x4b8/0x7d0
   tcp_close+0x23/0x90
   inet_release+0x93/0x100
   __sock_release+0x66/0x130
   sock_close+0x18/0x20
   __fput+0x1f0/0x4c0
   __x64_sys_close+0x55/0x90
  ...
  BUG: KASAN: slab-use-after-free in inet6_cleanup_sock+0x61/0x140
  Write of size 8 at addr ffff888016671b20 by task poc/111
  ...
  Call Trace:
   inet6_cleanup_sock+0x61/0x140
   inet6_sock_destruct+0x12/0x20
   __sk_destruct+0x4f/0x420
   inet_release+0x93/0x100
   __sock_release+0x66/0x130
   sock_close+0x18/0x20
   __fput+0x1f0/0x4c0
   __x64_sys_close+0x55/0x90
  ...
  Allocated by task 111:
   sk_prot_alloc+0x45/0x170
   sk_clone+0x49/0x970
   inet_csk_clone_lock+0x29/0x2c0
   tcp_create_openreq_child+0x2a/0x10a0
   tcp_v4_syn_recv_sock+0xd3/0x850
   tcp_v6_syn_recv_sock+0xc12/0xd80
   tcp_check_req+0x390/0x1080
   tcp_v4_rcv+0xc15/0x21c0
  ...
  Freed by task 14:
   slab_free_after_rcu_debug+0xd5/0x220
   rcu_core+0x4fe/0xe20
  ...
  The buggy address belongs to the object at ffff888016670e40
   which belongs to the cache TCPv6 of size 3328

Fix this by clearing ->pinet6 and ->ipv6_fl_list on the child, and by
returning early from inet6_cleanup_sock() when there is no ipv6_pinfo.
tcp_v6_mapped_child_init() sets both fields, so the v4-mapped path is not
affected. The converted listener keeps its own ipv6_pinfo, so there is
nothing to clear on the IPV6_ADDRFORM side.

Clearing ->pinet6 in tcp_disconnect() instead would keep this out of the
fast path, but it leaves the pointer NULL on a socket that still has a
file descriptor, and of the 120 inet6_sk() call sites only the two in
inet_ecn.h check it for NULL, so _all_ the others have to be found and
guarded first. At the point patched here the child is not in the ehash yet
and has no sk_socket. Once the two fields are cleared it is no different
from any other AF_INET child.

Fixes: d38afeec26ed ("tcp/udp: Call inet6_destroy_sock() in IPv6 sk->sk_destruct().")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
Changes in v2:
- Clear ->pinet6 and ->ipv6_fl_list right after the inet fields are set
instead of in an else arm of the opt_child_init test, so the child is
already consistent on the put_and_exit path and the existing test is left
untouched.
- Explain why this is not done in tcp_disconnect().
- Add the reproducer and the KASAN reports.
- v1: https://lore.kernel.org/all/antr7RCJAO578ZFW@v4bel/
---
 net/ipv4/tcp_ipv4.c | 8 ++++++++
 net/ipv6/af_inet6.c | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 190c7af4cf923a..302afe8ebcbcc3 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1714,6 +1714,14 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
 		inet_csk(newsk)->icsk_ext_hdr_len = inet_opt->opt.optlen;
 	atomic_set(&newinet->inet_id, get_random_u16());
 
+#if IS_ENABLED(CONFIG_IPV6)
+	/* Never inherit the listener's ipv6_pinfo; IPV6_ADDRFORM leaves it set
+	 * on an AF_INET socket.  tcp_v6_mapped_child_init() installs our own.
+	 */
+	newinet->pinet6 = NULL;
+	newinet->ipv6_fl_list = NULL;
+#endif
+
 	/* Set ToS of the new socket based upon the value of incoming SYN.
 	 * ECT bits are set later in tcp_init_transfer().
 	 */
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 282912a1199992..68b330f6941d04 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -479,6 +479,10 @@ void inet6_cleanup_sock(struct sock *sk)
 	struct sk_buff *skb;
 	struct ipv6_txoptions *opt;
 
+	/* AF_INET child of an IPV6_ADDRFORM'ed listener: nothing of its own. */
+	if (!np)
+		return;
+
 	/* Release rx options */
 
 	skb = xchg(&np->pktoptions, NULL);
-- 
2.43.0

[PATCH net v2 2/8] tcp: fix imbalanced icsk_accept_queue count in tcp_check_req()

[Hyunwoo Kim]

When TCP socket migration happens, tcp_v4_rcv() and tcp_v6_rcv() pass the
new listener to tcp_check_req(). The listener that counted the request is
still the one stored in req->rsk_listener.

The embryonic reset path passes @sk to inet_csk_reqsk_queue_drop(). After
migration this decrements the count on the new listener, which never
counted the request, and the count on the original listener is never
removed. The cited commit already changed the
inet_csk_reqsk_queue_drop_and_put() call in the same function to
req->rsk_listener. Do the same here.

Fixes: d4f2c86b2b7e ("tcp: Migrate TCP_NEW_SYN_RECV requests at receiving the final ACK.")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 net/ipv4/tcp_minisocks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 12254e6eb2f343..0c3b35a381e327 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -974,7 +974,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
 		tcp_reset(sk, skb);
 	}
 	if (!fastopen) {
-		bool unlinked = inet_csk_reqsk_queue_drop(sk, req);
+		bool unlinked = inet_csk_reqsk_queue_drop(req->rsk_listener, req);
 
 		if (unlinked)
 			__NET_INC_STATS(sock_net(sk), LINUX_MIB_EMBRYONICRSTS);
-- 
2.43.0

[PATCH net v2 3/8] ipv6: fix request socket use-after-free after IPV6_ADDRFORM

[Hyunwoo Kim]

IPV6_ADDRFORM turns an AF_INET6 TCP socket into an AF_INET one. It requires
the socket to be established, and a listener can get there with
connect(AF_UNSPEC) followed by connect(). Request sockets queued while it
was listening are still there: inet_csk_listen_stop() leaves them in the
ehash, and their timers only drop them while the socket is not listening,
so making it listen again keeps them alive.

A request that arrived over IPv6 was hashed with inet6_ehashfn(). Its child
is cloned from the converted socket and hashed with inet_ehashfn(), so it
belongs in a different bucket.

inet_ehash_insert() locks the child's bucket, warns about the mismatching
hashes, and replaces the request with the child in the request's own bucket
anyway. reqsk_queue_unlink() locks the bucket the request is really in, so
there is no synchronization between the two. Both can see the request still
hashed and both can drop the reference the ehash holds.

The extra put takes the request's refcount to zero too early, so it is
freed while it is still on the listener's accept queue. The listener is
then closed, and inet_csk_listen_stop() reads the freed request and writes
to it in reqsk_put().

In short:

  socket(AF_INET6) -> setsockopt(TCP_DEFER_ACCEPT, 120) -> bind -> listen
      // a native IPv6 client connects and sends nothing
      // the request stays in the bucket inet6_ehashfn() picked
  connect(AF_UNSPEC)                    // stop listening
  connect(a v4-mapped peer)             // become established
  setsockopt(IPV6_ADDRFORM, PF_INET)    // become an AF_INET socket
  connect(AF_UNSPEC), listen()          // listen again
      // the client sends data and the leftover request completes
  close()                               // use-after-free

KASAN log:

  BUG: KASAN: slab-use-after-free in inet_csk_listen_stop+0x1c2/0x760
  Write of size 4 at addr ffff888010b7cbe0 by task init/1
  ...
  Call Trace:
   inet_csk_listen_stop+0x1c2/0x760
   __tcp_close+0x6c1/0x7b0
   tcp_close+0x23/0x90
   inet_release+0x93/0x100
   __sock_release+0x66/0x130
   sock_close+0x18/0x20
   __fput+0x1f0/0x4c0
   fput_close_sync+0xd2/0x170
   __x64_sys_close+0x55/0x90
  ...
  Allocated by task 0:
   inet_reqsk_alloc+0x8c/0x320
   tcp_conn_request+0x324/0x11f0
   tcp_rcv_state_process+0x2ff/0x2cf0
   tcp_v6_do_rcv+0x326/0xc30
   tcp_v6_rcv+0x1e07/0x1e90
  ...
  Freed by task 0:
   slab_free_after_rcu_debug+0xc5/0x200
   rcu_core+0x4dc/0xd20
  ...
  Last potentially related work creation:
   kmem_cache_free+0x11d/0x5f0
   tcp_v6_rcv+0xb30/0x1e90
  ...
  The buggy address belongs to the object at ffff888010b7cb60
   which belongs to the cache request_sock_TCPv6 of size 352

Refuse the conversion if inet_csk_reqsk_queue_len() is not zero. Nothing
clears that counter when a socket stops listening or listens again, so it
still accounts for the requests left in the ehash. A socket that never
listened is not affected.

Reading the counter once is not enough. inet_csk_reqsk_queue_hash_add()
puts the request in the ehash first and bumps the counter second, so a
setsockopt that lands in between misses a request that is already
reachable. No new request is created once the socket stops listening, and
a SYN that found it while it was still listening is handled inside the RCU
read-side critical section the receive path holds. Waiting for one grace
period before reading again therefore leaves no request that is reachable
but not yet counted.

Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
Changes in v2:
- Read inet_csk_reqsk_queue_len() again after synchronize_rcu(). A request
  is put in the ehash before the counter is bumped, so v1 could be raced.
- Move the check below the TCP_ESTABLISHED and v4-mapped checks. In v1 a
  listener returned -EBUSY or -ENOTCONN depending on whether a peer had a
  half-open connection at that moment.
- Add the trigger sequence and the KASAN log to the commit message.
- v1: https://lore.kernel.org/all/20260817090319.3897799-2-imv4bel@gmail.com/
---
 net/ipv6/ipv6_sockglue.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index b4c977434c2e0a..6d2dd9ae46a964 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -587,6 +587,21 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 				break;
 			}
 
+			if (sk->sk_protocol == IPPROTO_TCP) {
+				if (inet_csk_reqsk_queue_len(sk)) {
+					retv = -EBUSY;
+					break;
+				}
+				/* A SYN that found this socket while it was
+				 * still listening may not be counted yet.
+				 */
+				synchronize_rcu();
+				if (inet_csk_reqsk_queue_len(sk)) {
+					retv = -EBUSY;
+					break;
+				}
+			}
+
 			__ipv6_sock_mc_close(sk);
 			__ipv6_sock_ac_close(sk);
 
-- 
2.43.0

[PATCH net v2 4/8] net: fix out-of-bounds write in sk_clone() racing with IPV6_ADDRFORM

[Hyunwoo Kim]

sk_clone() allocates the child from sk->sk_prot, and IPV6_ADDRFORM can
change sk_prot under it. The conversion requires the socket to be
established, and a listener gets there with connect(AF_UNSPEC) followed
by connect().

tcp_check_req() completes a request without the listener lock, so it can
run while the conversion is in progress. IPV6_ADDRFORM stores sk_prot
before icsk_af_ops, so tcp_check_req() can still call
tcp_v6_syn_recv_sock() once sk_prot is tcp_prot. The child then comes
from tcp_prot's slab while the AF_INET6 code treats it as a tcp6_sock.

tcp_inet6_sk() is a fixed offset into tcp6_sock, and in a child sized by
tcp_prot that offset is the end of the object. The ipv6_pinfo copy is
therefore a slab out-of-bounds write of sizeof(struct ipv6_pinfo) bytes
past the child.

The out-of-bounds address is also stored in the child's pinet6, so
everything that reaches the socket through inet6_sk() keeps writing
there. A request that arrived over IPv4 takes the same copy in
tcp_v6_mapped_child_init().

In short:

  client                              the socket owner

                                      socket(AF_INET6)
                                      setsockopt(TCP_DEFER_ACCEPT, 30)
                                      bind(), listen()
  connect(::1)
      // bare ACK deferred, request stays
  send()
      // tcp_check_req() ->
      //   picks tcp_v6_syn_recv_sock()
                                      connect(AF_UNSPEC)
                                      connect(::ffff:127.0.0.1)
                                      setsockopt(IPV6_ADDRFORM, PF_INET)
                                          // sk_prot = tcp_prot
      // sk_clone() -> child from the TCP slab
      // tcp_v6_syn_recv_sock() -> memcpy(ipv6_pinfo)
      // out-of-bounds write of 128 bytes

KASAN log:

  BUG: KASAN: slab-out-of-bounds in tcp_v6_syn_recv_sock+0x297/0xce0
  Write of size 128 at addr ffff888015b5f480 by task repro/161
  ...
  Call Trace:
   <IRQ>
   __asan_memcpy+0x3c/0x60
   tcp_v6_syn_recv_sock+0x297/0xce0
   tcp_check_req+0x374/0xff0
   tcp_v6_rcv+0xb9f/0x1e90
   ip6_protocol_deliver_rcu+0x1aa/0x870
   ip6_input_finish+0xac/0x1a0
   ip6_input+0xe5/0x490
   ipv6_rcv+0x2a0/0x3d0
  ...
  Allocated by task 161:
   sk_prot_alloc+0x45/0x170
   sk_clone+0x49/0x960
   inet_csk_clone_lock+0x29/0x2c0
   tcp_create_openreq_child+0x2a/0xf20
   tcp_v6_syn_recv_sock+0x14e/0xce0
   tcp_check_req+0x374/0xff0
   tcp_v6_rcv+0xb9f/0x1e90
  ...
  The buggy address belongs to the object at ffff888015b5e800
   which belongs to the cache TCP of size 3200
  The buggy address is located 0 bytes to the right of
   allocated 3200-byte region [ffff888015b5e800, ffff888015b5f480)

Checking sk_prot before the clone does not help. It can change between
that check and the read inside sk_clone(). Use sk_prot_creator instead.
It is set once in sk_alloc() and never changes, and the socket is
already freed back through it. No caller that replaces sk_prot installs
a proto with a larger obj_size than the creator, so the child gets the
size the parent object actually has.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
Changes in v2:
- Add the trigger sequence and the KASAN log to the commit message.
- v1: https://lore.kernel.org/all/20260817090319.3897799-3-imv4bel@gmail.com/
---
 net/core/sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 1ad41904db25b4..098e58b40f304b 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2479,7 +2479,7 @@ static void sk_init_common(struct sock *sk)
 struct sock *sk_clone(const struct sock *sk, const gfp_t priority,
 		      bool lock)
 {
-	struct proto *prot = READ_ONCE(sk->sk_prot);
+	struct proto *prot = sk->sk_prot_creator;
 	struct sk_filter *filter;
 	bool is_charged = true;
 	struct sock *newsk;
-- 
2.43.0

[PATCH net v2 5/8] tcp: do not inherit out_of_order_queue from parent

[Hyunwoo Kim]

A child gets a copy of the parent's out_of_order_queue, which can be non
empty when/if parent morphs from listener to active session. Parent and
child then point at the same rbtree.

The parent is no longer a listener, so inet_csk_reqsk_queue_add() forgets
the child immediately, and tcp_disconnect() frees the skbs the parent
still owns. The parent's own root and ooo_last_skb are left alone, so it
keeps using those skbs. That is a use-after-free, and the parent frees
them a second time when it closes.

In short:

  peers                               the socket owner

                                      socket(AF_INET6)
                                      setsockopt(TCP_DEFER_ACCEPT, 30)
                                      bind(), listen()
  connect(the listener)
      // bare ACK deferred, the request stays
                                      connect(AF_UNSPEC)
                                      connect(the new peer)
  send(the new connection)
      // it starts one byte past rcv_nxt, so it lands out of order
      tcp_data_queue()
        tcp_data_queue_ofo()  // the parent's queue fills up
  send(the old connection)
      tcp_check_req()
        tcp_v6_syn_recv_sock()
          tcp_create_openreq_child()  // the child gets the same rbtree
        inet_csk_complete_hashdance()
          inet_csk_reqsk_queue_add()
            inet_child_forget()
              tcp_disconnect()
                skb_rbtree_purge()  // the parent's skbs are freed
  send(the new connection)
      tcp_data_queue()
        tcp_data_queue_ofo()
          rb_link_node()  // use-after-free write

KASAN log:

  BUG: KASAN: slab-use-after-free in tcp_data_queue+0x1708/0x1df0
  Write of size 8 at addr ffff888012ad5408 by task repro/135
  ...
  Call Trace:
   <IRQ>
   tcp_data_queue+0x1708/0x1df0
   tcp_rcv_established+0x441/0x1830
   tcp_v4_do_rcv+0x47e/0x730
   tcp_v4_rcv+0x171c/0x2040
   ip_protocol_deliver_rcu+0x5c/0x280
   ip_local_deliver_finish+0x15a/0x2e0
   ip_local_deliver+0x107/0x370
   ip_rcv+0x3b1/0x3d0
  ...
  Allocated by task 135:
   __alloc_skb+0xd0/0x370
   alloc_skb_with_frags+0x7d/0x330
   sock_alloc_send_pskb+0x490/0x4e0
   raw_sendmsg+0xd52/0x1850
  ...
  Freed by task 133:
   kmem_cache_free+0x26f/0x5f0
   skb_rbtree_purge+0x73/0x90
   tcp_disconnect+0x1be/0xd60
   inet_child_forget+0x41/0x140
   inet_csk_complete_hashdance+0x4d0/0x520
   tcp_check_req+0x9f9/0xff0
   tcp_v6_rcv+0xb9f/0x1e90
  ...
  The buggy address belongs to the object at ffff888012ad5400
   which belongs to the cache skbuff_head_cache of size 232
  The buggy address is located 8 bytes inside of
   freed 232-byte region [ffff888012ad5400, ffff888012ad54e8)

We need to make sure this can not happen, by initializing the queue after
socket cloning.

Very similar to commit 8b485ce69876 ("tcp: do not inherit fastopen_req
from parent")

Fixes: 9f5afeae5152 ("tcp: use an RB tree for ooo receive queue")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
Changes in v2:
- Add the trigger sequence and the KASAN log to the commit message.
- v1: https://lore.kernel.org/all/20260817090319.3897799-4-imv4bel@gmail.com/
---
 net/ipv4/tcp_minisocks.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 0c3b35a381e327..7fe318d9e0aed2 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -591,6 +591,7 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
 	newtp->total_retrans = req->num_retrans;
 
 	tcp_init_xmit_timers(newsk);
+	newtp->out_of_order_queue = RB_ROOT;
 	WRITE_ONCE(newtp->write_seq, newtp->pushed_seq = treq->snt_isn + 1);
 
 	if (sock_flag(newsk, SOCK_KEEPOPEN))
-- 
2.43.0

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

[Hyunwoo Kim]

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>
---
 include/net/tcp.h    |  2 ++
 net/ipv4/tcp_input.c | 64 +++++++++++++++++++++++++-------------------
 net/ipv4/tcp_ipv4.c  | 44 ++++++++++++++++++++++++++++--
 net/ipv6/tcp_ipv6.c  | 44 ++++++++++++++++++++++++++++--
 4 files changed, 123 insertions(+), 31 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 2c5b889530b556..add438d6561be5 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -392,6 +392,8 @@ void tcp_write_timer_handler(struct sock *sk);
 void tcp_delack_timer_handler(struct sock *sk);
 int tcp_ioctl(struct sock *sk, int cmd, int *karg);
 enum skb_drop_reason tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb);
+enum skb_drop_reason tcp_rcv_listen_state_process(struct sock *sk,
+						  struct sk_buff *skb);
 void tcp_rcv_established(struct sock *sk, struct sk_buff *skb);
 void tcp_rcvbuf_grow(struct sock *sk, u32 newval);
 void tcp_rcv_space_adjust(struct sock *sk);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0f60a1dbf92746..77af18fba66b4d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -7141,6 +7141,42 @@ static void tcp_rcv_synrecv_state_fastopen(struct sock *sk)
 	tcp_rearm_rto(sk);
 }
 
+enum skb_drop_reason tcp_rcv_listen_state_process(struct sock *sk,
+						  struct sk_buff *skb)
+{
+	const struct tcphdr *th = tcp_hdr(skb);
+	SKB_DR(reason);
+
+	if (th->ack)
+		return SKB_DROP_REASON_TCP_FLAGS;
+
+	if (th->rst) {
+		SKB_DR_SET(reason, TCP_RESET);
+		goto discard;
+	}
+	if (th->syn) {
+		if (th->fin) {
+			SKB_DR_SET(reason, TCP_FLAGS);
+			goto discard;
+		}
+		/* It is possible that we process SYN packets from backlog,
+		 * so we need to make sure to disable BH and RCU right there.
+		 */
+		rcu_read_lock();
+		local_bh_disable();
+		inet_csk(sk)->icsk_af_ops->conn_request(sk, skb);
+		local_bh_enable();
+		rcu_read_unlock();
+
+		consume_skb(skb);
+		return 0;
+	}
+	SKB_DR_SET(reason, TCP_FLAGS);
+discard:
+	tcp_drop_reason(sk, skb, reason);
+	return 0;
+}
+
 /*
  *	This function implements the receiving procedure of RFC 793 for
  *	all states except ESTABLISHED and TIME_WAIT.
@@ -7152,7 +7188,6 @@ enum skb_drop_reason
 tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
-	struct inet_connection_sock *icsk = inet_csk(sk);
 	const struct tcphdr *th = tcp_hdr(skb);
 	struct request_sock *req;
 	int queued = 0;
@@ -7164,32 +7199,7 @@ tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 		goto discard;
 
 	case TCP_LISTEN:
-		if (th->ack)
-			return SKB_DROP_REASON_TCP_FLAGS;
-
-		if (th->rst) {
-			SKB_DR_SET(reason, TCP_RESET);
-			goto discard;
-		}
-		if (th->syn) {
-			if (th->fin) {
-				SKB_DR_SET(reason, TCP_FLAGS);
-				goto discard;
-			}
-			/* It is possible that we process SYN packets from backlog,
-			 * so we need to make sure to disable BH and RCU right there.
-			 */
-			rcu_read_lock();
-			local_bh_disable();
-			icsk->icsk_af_ops->conn_request(sk, skb);
-			local_bh_enable();
-			rcu_read_unlock();
-
-			consume_skb(skb);
-			return 0;
-		}
-		SKB_DR_SET(reason, TCP_FLAGS);
-		goto discard;
+		return tcp_rcv_listen_state_process(sk, skb);
 
 	case TCP_SYN_SENT:
 		tp->rx_opt.saw_tstamp = 0;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 302afe8ebcbcc3..2fa8958380a5b9 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1828,7 +1828,7 @@ u16 tcp_v4_get_syncookie(struct sock *sk, struct iphdr *iph,
 INDIRECT_CALLABLE_DECLARE(struct dst_entry *ipv4_dst_check(struct dst_entry *,
 							   u32));
 /* The socket must have it's spinlock held when we get
- * here, unless it is a TCP_LISTEN socket.
+ * here.
  *
  * We have a potential double-lock case here, so even when
  * doing backlog processing we use the BH locking scheme.
@@ -1906,6 +1906,46 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 	goto discard;
 }
 
+/* @sk is not locked here and can leave TCP_LISTEN; do not test sk_state. */
+static noinline int tcp_v4_rcv_listen(struct sock *sk, struct sk_buff *skb)
+{
+	enum skb_drop_reason reason;
+	struct sock *nsk;
+
+	reason = psp_sk_rx_policy_check(sk, skb);
+	if (reason)
+		goto err_discard;
+
+	if (tcp_checksum_complete(skb))
+		goto csum_err;
+
+	nsk = tcp_v4_cookie_check(sk, skb);
+	if (!nsk)
+		return 0;
+
+	if (nsk != sk) {
+		reason = tcp_child_process(sk, nsk, skb);
+		sock_put(nsk);
+	} else {
+		reason = tcp_rcv_listen_state_process(sk, skb);
+	}
+	if (!reason)
+		return 0;
+
+	tcp_v4_send_reset(sk, skb, sk_rst_convert_drop_reason(reason));
+discard:
+	sk_skb_reason_drop(sk, skb, reason);
+	return 0;
+
+csum_err:
+	reason = SKB_DROP_REASON_TCP_CSUM;
+	trace_tcp_bad_csum(skb);
+	TCP_INC_STATS(sock_net(sk), TCP_MIB_CSUMERRORS);
+err_discard:
+	TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
+	goto discard;
+}
+
 enum skb_drop_reason tcp_add_backlog(struct sock *sk, struct sk_buff *skb)
 {
 	u32 tail_gso_size, tail_gso_segs;
@@ -2243,7 +2283,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
 	skb->dev = NULL;
 
 	if (sk->sk_state == TCP_LISTEN) {
-		ret = tcp_v4_do_rcv(sk, skb);
+		ret = tcp_v4_rcv_listen(sk, skb);
 		goto put_and_return;
 	}
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 9e9155b1b3aa75..a2deda9a4258bc 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1556,7 +1556,7 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
 INDIRECT_CALLABLE_DECLARE(struct dst_entry *ipv4_dst_check(struct dst_entry *,
 							   u32));
 /* The socket must have it's spinlock held when we get
- * here, unless it is a TCP_LISTEN socket.
+ * here.
  *
  * We have a potential double-lock case here, so even when
  * doing backlog processing we use the BH locking scheme.
@@ -1704,6 +1704,46 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 	return 0;
 }
 
+/* @sk is not locked here and can leave TCP_LISTEN; do not test sk_state. */
+static noinline int tcp_v6_rcv_listen(struct sock *sk, struct sk_buff *skb)
+{
+	enum skb_drop_reason reason;
+	struct sock *nsk;
+
+	reason = psp_sk_rx_policy_check(sk, skb);
+	if (reason)
+		goto err_discard;
+
+	if (tcp_checksum_complete(skb))
+		goto csum_err;
+
+	nsk = tcp_v6_cookie_check(sk, skb);
+	if (!nsk)
+		return 0;
+
+	if (nsk != sk) {
+		reason = tcp_child_process(sk, nsk, skb);
+		sock_put(nsk);
+	} else {
+		reason = tcp_rcv_listen_state_process(sk, skb);
+	}
+	if (!reason)
+		return 0;
+
+	tcp_v6_send_reset(sk, skb, sk_rst_convert_drop_reason(reason));
+discard:
+	sk_skb_reason_drop(sk, skb, reason);
+	return 0;
+
+csum_err:
+	reason = SKB_DROP_REASON_TCP_CSUM;
+	trace_tcp_bad_csum(skb);
+	TCP_INC_STATS(sock_net(sk), TCP_MIB_CSUMERRORS);
+err_discard:
+	TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
+	goto discard;
+}
+
 static void tcp_v6_fill_cb(struct sk_buff *skb, const struct ipv6hdr *hdr,
 			   const struct tcphdr *th)
 {
@@ -1891,7 +1931,7 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
 	skb->dev = NULL;
 
 	if (sk->sk_state == TCP_LISTEN) {
-		ret = tcp_v6_do_rcv(sk, skb);
+		ret = tcp_v6_rcv_listen(sk, skb);
 		goto put_and_return;
 	}
 
-- 
2.43.0

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

[Hyunwoo Kim]

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().

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

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

[Paolo Abeni]

On 9/1/26 9:37 AM, 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().

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

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

Thanks,

Paolo

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

[Hyunwoo Kim]

On Tue, Sep 01, 2026 at 10:03:51AM +0200, Paolo Abeni wrote:
> On 9/1/26 9:37 AM, 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().
> 
> This looks like a more palatable approach: this patch in the current
> format looked way too invasive to me.
> 
> > 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

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

[Jakub Kicinski]

On Tue, 1 Sep 2026 10:03:51 +0200 Paolo Abeni wrote:
> > 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.

I likely lack context on this, but I was wondering whether we should
potentially disallow the transitions between listening and data sockets
instead of fixing these endless bugs?

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

[Eric Dumazet]

On Tue, Sep 1, 2026 at 5:51 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 1 Sep 2026 10:03:51 +0200 Paolo Abeni wrote:
> > > 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.
>
> I likely lack context on this, but I was wondering whether we should
> potentially disallow the transitions between listening and data sockets
> instead of fixing these endless bugs?

+2 I think I mentioned this at some point.

Same for IPV6_ADDRFORM : we should not allow transformed socket to
even use tcp_disconnect().

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

[Hyunwoo Kim]

On Tue, Sep 01, 2026 at 05:57:04PM +0200, Eric Dumazet wrote:
> On Tue, Sep 1, 2026 at 5:51 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Tue, 1 Sep 2026 10:03:51 +0200 Paolo Abeni wrote:
> > > > 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.
> >
> > I likely lack context on this, but I was wondering whether we should
> > potentially disallow the transitions between listening and data sockets
> > instead of fixing these endless bugs?
> 
> +2 I think I mentioned this at some point.
> 
> Same for IPV6_ADDRFORM : we should not allow transformed socket to
> even use tcp_disconnect().

So.. do you have a plan for this work?

Or should I send the v3 series first? I am still looking into related
problems, and it looks like this series covers most of the "important"
ones that have to be handled right away. What is left seems to be the
UDP side and other minor problems.


Best regards,
Hyuwnoo Kim

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

[Kuniyuki Iwashima]

On Thu, Sep 3, 2026 at 12:17 PM Hyunwoo Kim <imv4bel@gmail.com> wrote:
>
> On Tue, Sep 01, 2026 at 05:57:04PM +0200, Eric Dumazet wrote:
> > On Tue, Sep 1, 2026 at 5:51 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > >
> > > On Tue, 1 Sep 2026 10:03:51 +0200 Paolo Abeni wrote:
> > > > > 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.
> > >
> > > I likely lack context on this, but I was wondering whether we should
> > > potentially disallow the transitions between listening and data sockets
> > > instead of fixing these endless bugs?
> >
> > +2 I think I mentioned this at some point.
> >
> > Same for IPV6_ADDRFORM : we should not allow transformed socket to
> > even use tcp_disconnect().
>
> So.. do you have a plan for this work?

I'll post a patch to disallow the buggy transformation.
https://lore.kernel.org/netdev/CAAVpQUAWjBuF1Rgze0Vt_XvkUT=Ajw-F+Kh+s2iHk38c8iAapg@mail.gmail.com/

>
> Or should I send the v3 series first? I am still looking into related
> problems, and it looks like this series covers most of the "important"
> ones that have to be handled right away. What is left seems to be the
> UDP side and other minor problems.
>
>
> Best regards,
> Hyuwnoo Kim

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

[Hyunwoo Kim]

On Thu, Sep 03, 2026 at 12:28:15PM -0700, Kuniyuki Iwashima wrote:
> On Thu, Sep 3, 2026 at 12:17 PM Hyunwoo Kim <imv4bel@gmail.com> wrote:
> >
> > On Tue, Sep 01, 2026 at 05:57:04PM +0200, Eric Dumazet wrote:
> > > On Tue, Sep 1, 2026 at 5:51 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > > >
> > > > On Tue, 1 Sep 2026 10:03:51 +0200 Paolo Abeni wrote:
> > > > > > 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.
> > > >
> > > > I likely lack context on this, but I was wondering whether we should
> > > > potentially disallow the transitions between listening and data sockets
> > > > instead of fixing these endless bugs?
> > >
> > > +2 I think I mentioned this at some point.
> > >
> > > Same for IPV6_ADDRFORM : we should not allow transformed socket to
> > > even use tcp_disconnect().
> >
> > So.. do you have a plan for this work?
> 
> I'll post a patch to disallow the buggy transformation.
> https://lore.kernel.org/netdev/CAAVpQUAWjBuF1Rgze0Vt_XvkUT=Ajw-F+Kh+s2iHk38c8iAapg@mail.gmail.com/

Understood. I hope my Reported-by: will be included in that patch.

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

[Hyunwoo Kim]

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

[PATCH net v2 7/8] net: clear sk_tsq_flags in sk_clone()

[Hyunwoo Kim]

A socket returned by accept() can be freed while its fd is still open. A
setsockopt() on that fd then hits a use-after-free.

TCP_TSQ_DEFERRED owns a socket reference. tcp_tsq_handler() sets the bit
and calls sock_hold() when the socket is owned by user, and
tcp_release_cb() clears the bit and calls __sock_put(). sock_copy() gives
the child the bit but not the reference, so the __sock_put() that runs when
accept() locks and unlocks the child has nothing to pair with. The socket
is freed by the next put, which in the log below came from a timer.

A listener can be holding this bit. An established socket becomes a
listener again through connect(AF_UNSPEC) and listen(), and
tcp_clear_xmit_timers() only calls hrtimer_try_to_cancel(), so a pacing
callback that is already running survives. That callback sets the bit while
accept() holds the socket lock, and in the same window the child is created
in the TCP_NEW_SYN_RECV branch of tcp_v4_rcv(), which does not take the
listener lock.

All seven bits in sk_tsq_flags describe work pending on the parent, and
four of them own a reference. They are set in four different places, so
clear the whole word in sk_clone().

In short:

  socket(AF_INET) -> setsockopt(SO_MAX_PACING_RATE, 100000)
                  -> setsockopt(TCP_MAXSEG, 1200) -> bind -> connect(peer)
  send(64KB)                          // arms the pacing timer, sock_hold()
  connect(AF_UNSPEC)                  // stop being connected
  listen()                            // become a listener again
  setsockopt(TCP_DEFER_ACCEPT)
      // another socket connects, and with TCP_DEFER_ACCEPT there is no
      // child yet
  accept()                            // the child is created when one byte
                                      // arrives. while accept() holds the
                                      // lock the pacing callback sets the
                                      // bit, and the child is cloned by
                                      // the receive path, which does not
                                      // take the listener lock
      // a timer on the child does the last put and the socket is freed
  setsockopt(accepted, TCP_NODELAY)   // use-after-free

KASAN log:

  BUG: KASAN: slab-use-after-free in sock_common_setsockopt+0x44/0x80
  Read of size 8 at addr ffff88800e2ab668 by task repro/94
  ...
  Call Trace:
   sock_common_setsockopt+0x44/0x80
   do_sock_setsockopt+0x15e/0x2b0
   __sys_setsockopt+0x9e/0xe0
   __x64_sys_setsockopt+0x64/0x80
  ...
  Allocated by task 95:
   sk_prot_alloc+0x45/0x170
   sk_clone+0x49/0x960
   inet_csk_clone_lock+0x29/0x2c0
   tcp_create_openreq_child+0x2a/0xf20
   tcp_v4_syn_recv_sock+0xd3/0x7e0
   tcp_check_req+0x374/0xff0
   tcp_v4_rcv+0xc2d/0x2040
  ...
  Freed by task 0:
   slab_free_after_rcu_debug+0xc5/0x200
   rcu_core+0x4de/0xd30
  ...
  Last potentially related work creation:
   kmem_cache_free+0x11d/0x5f0
   __sk_destruct+0x29a/0x3d0
   call_timer_fn+0x12f/0x3f0
   __run_timers+0x4a4/0x5e0
  ...
  The buggy address belongs to the object at ffff88800e2ab640
   which belongs to the cache TCP of size 3200

Fixes: 73a6bab5aa2a ("tcp: switch pacing timer to softirq based hrtimer")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 net/core/sock.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/sock.c b/net/core/sock.c
index 098e58b40f304b..06fbb19824e267 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2533,6 +2533,7 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority,
 	newsk->sk_reserved_mem  = 0;
 	DEBUG_NET_WARN_ON_ONCE(newsk->sk_drop_counters);
 	sk_drops_reset(newsk);
+	newsk->sk_tsq_flags	= 0;
 	newsk->sk_send_head	= NULL;
 	newsk->sk_userlocks	= sk->sk_userlocks & ~SOCK_BINDPORT_LOCK;
 	atomic_set(&newsk->sk_zckey, 0);
-- 
2.43.0

[PATCH net v2 8/8] tcp: do not inherit retransmit state from parent

[Hyunwoo Kim]

A child gets a copy of the parent's retransmit state when it is cloned.
On a listener it is all zero, which is why commit eb2c80ca87b1 ("tcp: do
not clear packets_out in tcp_create_openreq_child()") and
commit 5c701549c9a6 ("tcp: move retrans_out, sacked_out, tlp_high_seq,
last_oow_ack_time init to tcp_disconnect()") dropped the initialization
of packets_out, retrans_out and sacked_out here. lost_out,
retransmit_skb_hint and highest_sack have never been cleared here.

The parent can morph from listener to active session while a request is
still being processed, and connect(AF_UNSPEC) followed by connect()
gets it there. tcp_check_req() does not hold the listener lock, so
nothing pins the parent's state between the TCP_LISTEN test and the
clone. The child then copies the counters and the two pointers into the
parent's retransmit queue.

sk_clone() sets sk_send_head to NULL, and tcp_rtx_queue is unioned with
it, so the child's retransmit queue is empty. That does not help.
packets_out is set again as soon as the child sends anything, and
tcp_xmit_retransmit_queue() picks the copied hint over the queue head.
When the parent disconnects, tcp_write_queue_purge() frees those skbs,
but the pointers the child copied are left alone. The child then gets
an ACK, enters the retransmit path and writes into a freed skb.

In short:

  socket(AF_INET) -> setsockopt(TCP_DEFER_ACCEPT, 30) -> bind -> listen
      // a client connects and sends one byte. the kernel processes
      // that segment while the steps below run
  connect(AF_UNSPEC)              // stop listening
  connect(peer)                   // become an active session
  send() repeatedly               // lower IP_TTL so the peer's IP_MINTTL
                                  // drops most of them, and let one
                                  // through to get a SACK
      // parent: packets_out 6, sacked_out 1, lost_out 5, retrans_out 1
      // retransmit_skb_hint points at an skb in the parent's queue
      // the leftover request completes and copies this state
  connect(AF_UNSPEC)              // those skbs are freed
  listen()                        // or the child is dropped
  accept()
      // the child sends data, one segment is lost, and the ACK that
      // comes back takes the retransmit path to the copied hint

KASAN log:

  BUG: KASAN: slab-use-after-free in __pskb_trim_head+0x66b/0x900
  Write of size 16 at addr ffff888008141530 by task repro/76
  ...
  Call Trace:
   __pskb_trim_head+0x66b/0x900
   tcp_trim_head+0x69/0x540
   __tcp_retransmit_skb+0x14e/0x26b0
   tcp_retransmit_skb+0x1b/0x250
   tcp_xmit_retransmit_queue.part.0+0x3b1/0x970
   tcp_ack+0x3382/0x7430
   tcp_rcv_established+0x631/0x3a00
   tcp_v4_do_rcv+0x449/0x960
   __release_sock+0x1f2/0x2a0
   release_sock+0x176/0x1d0
   tcp_sendmsg+0x30/0x40
   __sys_sendto+0x316/0x380
   __x64_sys_sendto+0xdb/0x1b0
  ...
  Allocated by task 76:
   __alloc_skb+0x11e/0x890
   tcp_stream_alloc_skb+0x2c/0x5c0
   tcp_sendmsg_locked+0x1377/0x3df0
   tcp_sendmsg+0x26/0x40
   __sys_sendto+0x316/0x380
  ...
  Freed by task 80:
   skb_release_data+0x554/0x810
   __kfree_skb+0x42/0x60
   tcp_write_queue_purge+0x6ef/0xf40
   tcp_disconnect+0x2fc/0x1e10
   __inet_stream_connect+0x6d0/0xdf0
   inet_stream_connect+0x52/0xa0
   __sys_connect+0xfc/0x130
  ...
  The buggy address belongs to the object at ffff888008141380
   which belongs to the cache skbuff_small_head of size 704

We need to make sure this can not happen, by clearing them after socket
cloning. A listener always has them zero, so an ordinary passive open is
not affected. Clearing only the pointers is not enough: the counters
would then describe a retransmit queue the child does not have, and
tcp_fastretrans_alert() and tcp_retransmit_timer() warn.

Very similar to commit 8b485ce69876 ("tcp: do not inherit fastopen_req
from parent")

Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 net/ipv4/tcp_minisocks.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 7fe318d9e0aed2..2fde196dd302f5 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -660,6 +660,12 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
 	tcp_ecn_openreq_child(newsk, req, skb);
 	newtp->fastopen_req = NULL;
 	RCU_INIT_POINTER(newtp->fastopen_rsk, NULL);
+	newtp->packets_out = 0;
+	newtp->retrans_out = 0;
+	newtp->sacked_out = 0;
+	newtp->lost_out = 0;
+	newtp->retransmit_skb_hint = NULL;
+	newtp->highest_sack = NULL;
 
 	newtp->bpf_chg_cc_inprogress = 0;
 	tcp_bpf_clone(sk, newsk);
-- 
2.43.0