Netdev List
 help / color / mirror / Atom feed
* [PATCH net v3 0/2] mptcp: fix request migration ownership
@ 2026-08-06 11:14 Ren Wei
  2026-08-06 11:14 ` [PATCH net v3 1/2] mptcp: hold MP_JOIN msk ref when cloning reqsk Ren Wei
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ren Wei @ 2026-08-06 11:14 UTC (permalink / raw)
  To: netdev, mptcp
  Cc: matttbe, martineau, geliang, davem, edumazet, kuba, pabeni, horms,
	ncardwell, kuniyu, daniel, kafai, kylebot, david.lee, vega,
	caoruide123, weir, sashiko-bot

From: Ruide Cao <caoruide123@gmail.com>

Hi,

TCP request migration clones pending requests with inet_reqsk_clone().
Some MPTCP request fields carry ownership which cannot be transferred by
a plain byte copy.

For MP_JOIN requests, subflow_req->msk holds a socket reference.  The
clone inherits the pointer without acquiring its own reference, so the
original and cloned requests can drop the same reference.  Patch 1 lets
the cloned request take an additional reference.

For MP_CAPABLE requests, token_node belongs to the original request and
is hashed in the token table.  Copying the node gives the clone invalid
hash state, while concurrent request migration and RX processing can
observe different owners in the ehash and token tables.  Patch 2 moves
token-table ownership from the original request to the clone under the
bucket lock, and makes token acceptance and destruction tolerate a token
that has already been removed.

--------------------
Changes in v3:

- Split MP_JOIN and MP_CAPABLE into two patches.
- Reworked MP_CAPABLE token migration to happen during MPTCP request
  cloning.
- Made MP_CAPABLE accept/destroy paths tolerant of already moved or
  removed request tokens.
- Added a packetdrill MP_CAPABLE reproducer and decoded warning.
- Dropped the redundant IPPROTO_TCP guard around the direct MPTCP clone
  call, as request migration is TCP-only.
- Based v3 on the latest net tree rather than the current mptcp_net-next
  export, which has not yet merged commit a0ab2ba83e35 ("tcp: fix TFO
  max_qlen accounting across reuseport migration") and therefore lacks
  the overlapping inet_reqsk_clone() changes.
- v2 link:
  https://lore.kernel.org/all/40fd38e7a368e5b7bc9bc83364a32241f977d53f.1778404619.git.caoruide123@gmail.com/

Changes in v2:

- drop the generic request_sock clone callback
- call MPTCP directly from inet_reqsk_clone() under the TCP protocol
  check
- keep cloned MP_JOIN requests holding an msk reference
- clear raw-copied MP_CAPABLE token hash state in the clone
- move MP_CAPABLE token ownership only after successful req migration
- avoid exposing token internals to inet_connection_sock.c
- update the commit message accordingly
- v1 link:
  https://lore.kernel.org/all/86e2514b533bf4d55d4aa2fdbf1404022e8c9430.1776149210.git.caoruide123@gmail.com/

------------------------------

// poc for MP_JOIN:

// Minimal reproducer for a stale subflow_req->msk after reqsk migration.
--tolerance_usecs=200000
--non_fatal=packet

`sysctl -q net.mptcp.enabled=1
sysctl -q net.ipv4.tcp_migrate_req=1
sysctl -q net.ipv4.tcp_synack_retries=1`

// Listener A and the owning MPTCP connection.
+0     socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3
+0     setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0     setsockopt(3, SOL_SOCKET, SO_REUSEPORT, [1], 4) = 0
+0     bind(3, ..., ...) = 0
+0     listen(3, 8) = 0

+0.0   <  addr[caddr0] > addr[saddr0]  S   0:0(0)         win 65535  <mss 1460, sackOK, TS val 1000 ecr 0,    nop, wscale 8, mpcapable v1 flags[flag_h] nokey>
+0.0   >                               S.  0:0(0)  ack 1             <mss 1460, sackOK, TS val 1000 ecr 1000, nop, wscale 8, mpcapable v1 flags[flag_h] key[skey]>
+0.1   <                                .  1:1(0)  ack 1  win 256    <nop, nop, TS val 1000 ecr 1000, mpcapable v1 flags[flag_h] key[ckey=2, skey]>
+0     accept(3, ..., ...) = 4

// Make the MPTCP socket fully established so it accepts MP_JOIN.
+0.1   <                               P.  1:3(2)  ack 1  win 256    <nop, nop, TS val 1001 ecr 1000, mpcapable v1 flags[flag_h] key[skey, ckey] mpcdatalen 2, nop, nop>
+0.0   >                                .  1:1(0)  ack 3             <nop, nop, TS val 1001 ecr 1001, dss dack8=3 dll=0 nocs>

// Leave exactly one MP_JOIN request half-open.
+0.1   <  addr[caddr1] > addr[saddr0]  S   0:0(0)         win 65535  <mss 1460, sackOK, TS val 2000 ecr 0,    nop, wscale 8, mp_join_syn address_id=1 token=sha256_32(skey)>
+0.0   >                               S.  0:0(0)  ack 1             <mss 1460, sackOK, TS val 2000 ecr 2000, nop, wscale 8, mp_join_syn_ack address_id=0 sender_hmac=auto>

// Listener B joins the reuseport group, then A is closed. The next request
// timer clones and migrates the half-open MP_JOIN request to B.
+0.1   socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 5
+0     setsockopt(5, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0     setsockopt(5, SOL_SOCKET, SO_REUSEPORT, [1], 4) = 0
+0     bind(5, ..., ...) = 0
+0     listen(5, 8) = 0
+0     close(3) = 0

// Wait past the first SYN+ACK RTO, then release the owning MPTCP socket.
+1.5   setsockopt(4, SOL_SOCKET, SO_LINGER, {onoff=1, linger=0}, 8) = 0
+0     close(4) = 0

// The migrated request expires on its next timer and its destructor uses msk.
+4.0   `true`

------------------------------
crash log of MP_JOIN
[  280.449259] [      C0] BUG: KASAN: slab-use-after-free in subflow_req_destructor (net/mptcp/subflow.c:45)
[  280.449417] [      C0] Write of size 4 at addr ff1100010e008d40 by task swapper/0/0
[  280.449525] [      C0] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 7.2.0-rc5-00353-gc27e36054537 #10 PREEMPT(full)

[  280.449637] [      C0] Call Trace:
[  280.450422] [      C0]  subflow_req_destructor (net/mptcp/subflow.c:45)
[  280.450504] [      C0]  subflow_v4_req_destructor (net/mptcp/subflow.c:694)
[  280.450581] [      C0]  __reqsk_free (net/ipv4/inet_connection_sock.c:906)
[  280.450681] [      C0]  reqsk_timer_handler (include/net/request_sock.h:137 net/ipv4/inet_connection_sock.c:1147)

[  280.454937] [      C0] Allocated by task 10014:
[  280.455381] [      C0]  sk_prot_alloc (net/core/sock.c:2246)
[  280.455516] [      C0]  sk_clone (net/core/sock.c:2488)
[  280.455611] [      C0]  mptcp_sk_clone_init (include/net/sock.h:1848 net/mptcp/protocol.c:3564)
[  280.455683] [      C0]  subflow_syn_recv_sock (net/mptcp/subflow.c:883)
[  280.455772] [      C0]  tcp_check_req (net/ipv4/tcp_minisocks.c:934)

[  280.457177] [      C0] Freed by task 0:
[  280.457603] [      C0]  slab_free_after_rcu_debug (include/linux/kasan.h:235 mm/slub.c:2677 mm/slub.c:6439)
[  280.457688] [      C0]  rcu_core (kernel/rcu/tree.c:2645 kernel/rcu/tree.c:2897)

[  280.458199] [      C0] Last potentially related work creation:
[  280.458426] [      C0]  kmem_cache_free (mm/slub.c:2638 mm/slub.c:6377 mm/slub.c:6504)
[  280.458518] [      C0]  __sk_destruct (net/core/sock.c:2289 net/core/sock.c:2391)
[  280.458607] [      C0]  sk_destruct (net/core/sock.c:2419)
[  280.458700] [      C0]  __sk_free (net/core/sock.c:2430)
[  280.458793] [      C0]  sk_free (net/core/sock.c:2441)
[  280.458885] [      C0]  mptcp_close (include/net/sock.h:2020 net/mptcp/protocol.c:3399)
[  280.458969] [      C0]  inet_release (net/ipv4/af_inet.c:442)

[  280.459536] [      C0] The buggy address belongs to the cache MPTCP of size 2968
[  280.459593] [      C0] The buggy address is 128 bytes inside a freed 2968-byte region



------------------------------

MP_CAPABLE packetdrill reproducer:

// Reproducer for MP_CAPABLE request token ownership during TCP req migration.
//
// The first listener owns the request created by the MP_CAPABLE SYN.  A second
// SO_REUSEPORT listener is added only after that SYN, then the first listener is
// closed.  The SYN+ACK retransmission timer migrates the request to the second
// listener, and a later request timer destroys the migrated request.
//
// On a vulnerable kernel, inet_reqsk_clone() raw-copies token_node.  The clone
// is not the token table owner, so destroying the migrated request triggers the
// MPTCP token ownership bug.
--tolerance_usecs=250000

+0     `sysctl -q net.mptcp.enabled=1`
+0     `sysctl -q net.ipv4.tcp_migrate_req=1`
+0     `sysctl -q net.ipv4.tcp_synack_retries=2`
+0     `sysctl -q net.ipv4.tcp_timestamps=1`
+0     `sysctl -q kernel.panic_on_warn=0`
+0     `sysctl -q kernel.panic_on_oops=0`
+0     `ip tcp_metrics flush all >/dev/null 2>&1 || true`
+0     `tc qdisc replace dev tun0 root pfifo >/dev/null 2>&1 || true`

+0     socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3
+0     setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0     setsockopt(3, SOL_SOCKET, SO_REUSEPORT, [1], 4) = 0
+0     getsockopt(3, SOL_TCP, TCP_IS_MPTCP, [1], [4]) = 0
+0     bind(3, ..., ...) = 0
+0     listen(3, 1) = 0

+0       <  S   0:0(0)         win 32792  <mss 1000, sackOK, nop, nop, nop, wscale 7, mpcapable v1 flags[flag_h] nokey>
+0       >  S.  0:0(0)  ack 1             <mss 1460, nop, nop, sackOK, nop, wscale 9, mpcapable v1 flags[flag_h] key[skey]>

+0     socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 4
+0     setsockopt(4, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0     setsockopt(4, SOL_SOCKET, SO_REUSEPORT, [1], 4) = 0
+0     getsockopt(4, SOL_TCP, TCP_IS_MPTCP, [1], [4]) = 0
+0     bind(4, ..., ...) = 0
+0     listen(4, 1) = 0

+0     close(3) = 0

// Let the retransmission timer migrate the request to fd 4 and let the migrated
// request expire.  On vulnerable kernels its raw-copied token_node is not the
// token-table owner, so the request timer trips the token owner assertion.
+8.0   `true`

+0     close(4) = 0

------------------------------
decoded warning from MP_CAPABLE reproducer:

[  314.661418] [      C2] ------------[ cut here ]------------
[  314.661636] [      C2] WARNING: net/mptcp/token.c:364 at mptcp_token_destroy_request+0x2b0/0x330, CPU#2: swapper/2/0
[  314.661931] [      C2] CPU: 2 UID: 0 PID: 0 Comm: swapper/2 Not tainted 7.2.0-rc5-00353-gc27e36054537 #10 PREEMPT(full)
[  314.662092] [      C2] RIP: 0010:mptcp_token_destroy_request (build/../net/mptcp/token.c:364 (discriminator 1))
[  314.663001] [      C2] Call Trace:
[  314.663043] [      C2]  <IRQ>
[  314.663107] [      C2]  subflow_v4_req_destructor (build/../net/mptcp/subflow.c:694)
[  314.663213] [      C2]  __reqsk_free (build/../net/ipv4/inet_connection_sock.c:906)
[  314.663325] [      C2]  reqsk_timer_handler (build/../include/net/request_sock.h:137 build/../net/ipv4/inet_connection_sock.c:1147)
[  314.663762] [      C2]  call_timer_fn (build/../kernel/time/timer.c:1748)
[  314.668915] [      C2] ---[ end trace 0000000000000000 ]---


Ruide Cao (2):
  mptcp: hold MP_JOIN msk ref when cloning reqsk
  mptcp: fix MP_CAPABLE token migration when cloning reqsk

 include/net/mptcp.h             |  7 ++++
 net/ipv4/inet_connection_sock.c |  4 +++
 net/mptcp/protocol.c            | 12 ++++---
 net/mptcp/protocol.h            |  4 ++-
 net/mptcp/subflow.c             | 13 +++++++
 net/mptcp/token.c               | 61 +++++++++++++++++++++++++++++----
 net/mptcp/token_test.c          |  4 +--
 7 files changed, 90 insertions(+), 15 deletions(-)


base-commit: 44871eadd07a7f004aa00cb87399461eea08c630
-- 
2.43.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net v3 1/2] mptcp: hold MP_JOIN msk ref when cloning reqsk
  2026-08-06 11:14 [PATCH net v3 0/2] mptcp: fix request migration ownership Ren Wei
@ 2026-08-06 11:14 ` Ren Wei
  2026-08-06 11:14 ` [PATCH net v3 2/2] mptcp: fix MP_CAPABLE token migration " Ren Wei
  2026-08-06 12:43 ` [PATCH net v3 0/2] mptcp: fix request migration ownership Matthieu Baerts
  2 siblings, 0 replies; 5+ messages in thread
From: Ren Wei @ 2026-08-06 11:14 UTC (permalink / raw)
  To: netdev, mptcp
  Cc: matttbe, martineau, geliang, davem, edumazet, kuba, pabeni, horms,
	ncardwell, kuniyu, daniel, kafai, kylebot, david.lee, vega,
	caoruide123, weir, sashiko-bot

From: Ruide Cao <caoruide123@gmail.com>

TCP request migration clones pending request sockets with
inet_reqsk_clone(). For MPTCP MP_JOIN requests this byte-copies
subflow_req->msk, but the clone does not take its own reference.

The original and cloned request can consequently drop the same msk
reference from subflow_req_destructor(), leaving one request with a
dangling pointer after the other is released.

Add an MPTCP clone helper on the TCP request migration path and let
cloned subflow requests grab their own msk reference when one is
present.  The clone's normal destructor balances this reference on both
successful migration and failed clone/insert paths.

Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
Cc: stable@vger.kernel.org
Reported-by: Kyle Zeng <kylebot@openai.com>
Reported-by: David Lee <david.lee@trailofbits.com>
Closes: https://lore.kernel.org/all/20260804095051.715355-1-david.lee@trailofbits.com/
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Ruide Cao <caoruide123@gmail.com>
Signed-off-by: Ren Wei <weir@nebusec.ai>
---
 include/net/mptcp.h             |  7 +++++++
 net/ipv4/inet_connection_sock.c |  4 ++++
 net/mptcp/subflow.c             | 11 +++++++++++
 3 files changed, 22 insertions(+)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 71b9fc5a5796..0a02ac1ed22d 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -223,6 +223,8 @@ int mptcp_subflow_init_cookie_req(struct request_sock *req,
 struct request_sock *mptcp_subflow_reqsk_alloc(const struct request_sock_ops *ops,
 					       struct sock *sk_listener,
 					       bool attach_listener);
+void mptcp_subflow_reqsk_clone(struct request_sock *req,
+			       struct request_sock *new_req);
 
 __be32 mptcp_get_reset_option(const struct sk_buff *skb);
 
@@ -309,6 +311,11 @@ static inline struct request_sock *mptcp_subflow_reqsk_alloc(const struct reques
 	return NULL;
 }
 
+static inline void mptcp_subflow_reqsk_clone(struct request_sock *req,
+					     struct request_sock *new_req)
+{
+}
+
 static inline __be32 mptcp_reset_option(const struct sk_buff *skb)  { return htonl(0u); }
 
 static inline void mptcp_active_detect_blackhole(struct sock *sk, bool expired) { }
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 6257459bcee2..896f472dcba2 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -21,6 +21,7 @@
 #include <net/xfrm.h>
 #include <net/tcp.h>
 #include <net/tcp_ecn.h>
+#include <net/mptcp.h>
 #include <net/sock_reuseport.h>
 #include <net/addrconf.h>
 
@@ -961,6 +962,9 @@ static struct request_sock *inet_reqsk_clone(struct request_sock *req,
 		rcu_assign_pointer(tcp_sk(nreq->sk)->fastopen_rsk, nreq);
 	}
 
+	if (rsk_is_mptcp(req))
+		mptcp_subflow_reqsk_clone(req, nreq);
+
 	return nreq;
 }
 
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 8e386899ceb9..be7260821566 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -47,6 +47,17 @@ static void subflow_req_destructor(struct request_sock *req)
 	mptcp_token_destroy_request(req);
 }
 
+void mptcp_subflow_reqsk_clone(struct request_sock *req,
+			       struct request_sock *new_req)
+{
+	struct mptcp_subflow_request_sock *subflow_req;
+
+	subflow_req = mptcp_subflow_rsk(new_req);
+
+	if (subflow_req->msk)
+		sock_hold((struct sock *)subflow_req->msk);
+}
+
 static void subflow_generate_hmac(u64 key1, u64 key2, u32 nonce1, u32 nonce2,
 				  void *hmac)
 {
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net v3 2/2] mptcp: fix MP_CAPABLE token migration when cloning reqsk
  2026-08-06 11:14 [PATCH net v3 0/2] mptcp: fix request migration ownership Ren Wei
  2026-08-06 11:14 ` [PATCH net v3 1/2] mptcp: hold MP_JOIN msk ref when cloning reqsk Ren Wei
@ 2026-08-06 11:14 ` Ren Wei
  2026-08-06 12:43 ` [PATCH net v3 0/2] mptcp: fix request migration ownership Matthieu Baerts
  2 siblings, 0 replies; 5+ messages in thread
From: Ren Wei @ 2026-08-06 11:14 UTC (permalink / raw)
  To: netdev, mptcp
  Cc: matttbe, martineau, geliang, davem, edumazet, kuba, pabeni, horms,
	ncardwell, kuniyu, daniel, kafai, kylebot, david.lee, vega,
	caoruide123, weir, sashiko-bot

From: Ruide Cao <caoruide123@gmail.com>

TCP request migration clones pending request sockets with
inet_reqsk_clone().  For MPTCP MP_CAPABLE requests this byte-copies the
token_node hlist state into the clone even though the token table still
names the original request.

Moving the token only after inet_ehash_insert() succeeds leaves a window
where the cloned request is already globally visible from the ehash but
the token table still points at the original request.  Reordering the
move after clone but before ehash exposure closes that window, but
concurrent RX can still race on the old request and observe that the
token was already moved.

Move MP_CAPABLE token request ownership during MPTCP request cloning
under the token bucket lock.  Make mptcp_token_accept() and
mptcp_token_destroy_request() re-check token_node under the same lock and
treat an already moved or removed request as a normal race instead of
warning.

If the passive MP_CAPABLE socket cannot claim the token, fail
mptcp_sk_clone_init() and let the subflow fall back instead of
installing a socket with mismatched token ownership.

Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/86e2514b533bf4d55d4aa2fdbf1404022e8c9430.1776149210.git.caoruide123%40gmail.com
Signed-off-by: Ruide Cao <caoruide123@gmail.com>
Signed-off-by: Ren Wei <weir@nebusec.ai>
---
 net/mptcp/protocol.c   | 12 +++++----
 net/mptcp/protocol.h   |  4 ++-
 net/mptcp/subflow.c    |  2 ++
 net/mptcp/token.c      | 61 +++++++++++++++++++++++++++++++++++++-----
 net/mptcp/token_test.c |  4 +--
 5 files changed, 68 insertions(+), 15 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index ca644ec53eed..07eaa6858d05 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3608,17 +3608,19 @@ struct sock *mptcp_sk_clone_init(const struct sock *sk,
 	 */
 	mptcp_set_state(nsk, TCP_ESTABLISHED);
 
+	if (!mptcp_token_accept(subflow_req, msk)) {
+		mptcp_release_sched(msk);
+		inet_csk_prepare_forced_close(nsk);
+		tcp_done(nsk);
+		return NULL;
+	}
+
 	/* The msk maintain a ref to each subflow in the connections list */
 	WRITE_ONCE(msk->first, ssk);
 	subflow = mptcp_subflow_ctx(ssk);
 	list_add(&subflow->node, &msk->conn_list);
 	sock_hold(ssk);
 
-	/* new mpc subflow takes ownership of the newly
-	 * created mptcp socket
-	 */
-	mptcp_token_accept(subflow_req, msk);
-
 	/* set msk addresses early to ensure mptcp_pm_get_local_id()
 	 * uses the correct data
 	 */
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 4a2d40cd7b13..c58cfb1326b7 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1074,9 +1074,11 @@ static inline void mptcp_token_init_request(struct request_sock *req)
 }
 
 int mptcp_token_new_request(struct request_sock *req);
+void mptcp_token_move_request(struct request_sock *req,
+			      struct request_sock *new_req);
 void mptcp_token_destroy_request(struct request_sock *req);
 int mptcp_token_new_connect(struct sock *ssk);
-void mptcp_token_accept(struct mptcp_subflow_request_sock *r,
+bool mptcp_token_accept(struct mptcp_subflow_request_sock *r,
 			struct mptcp_sock *msk);
 bool mptcp_token_exists(u32 token);
 struct mptcp_sock *mptcp_token_get_sock(struct net *net, u32 token);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index be7260821566..65d876a08233 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -56,6 +56,8 @@ void mptcp_subflow_reqsk_clone(struct request_sock *req,
 
 	if (subflow_req->msk)
 		sock_hold((struct sock *)subflow_req->msk);
+
+	mptcp_token_move_request(req, new_req);
 }
 
 static void subflow_generate_hmac(u64 key1, u64 key2, u32 nonce1, u32 nonce2,
diff --git a/net/mptcp/token.c b/net/mptcp/token.c
index f1a50f367add..24877a2c3772 100644
--- a/net/mptcp/token.c
+++ b/net/mptcp/token.c
@@ -180,6 +180,36 @@ int mptcp_token_new_connect(struct sock *ssk)
 	return 0;
 }
 
+void mptcp_token_move_request(struct request_sock *req,
+			      struct request_sock *new_req)
+{
+	struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
+	struct mptcp_subflow_request_sock *new_subflow_req;
+	struct mptcp_subflow_request_sock *pos;
+	struct token_bucket *bucket;
+
+	new_subflow_req = mptcp_subflow_rsk(new_req);
+
+	if (hlist_nulls_unhashed_lockless(&subflow_req->token_node)) {
+		mptcp_token_init_request(new_req);
+		return;
+	}
+
+	bucket = token_bucket(subflow_req->token);
+	spin_lock_bh(&bucket->lock);
+	if (hlist_nulls_unhashed(&subflow_req->token_node)) {
+		mptcp_token_init_request(new_req);
+	} else {
+		pos = __token_lookup_req(bucket, subflow_req->token);
+		if (pos == subflow_req)
+			hlist_nulls_replace_init_rcu(&subflow_req->token_node,
+						     &new_subflow_req->token_node);
+		else
+			mptcp_token_init_request(new_req);
+	}
+	spin_unlock_bh(&bucket->lock);
+}
+
 /**
  * mptcp_token_accept - replace a req sk with full sock in token hash
  * @req: the request socket to be removed
@@ -187,24 +217,36 @@ int mptcp_token_new_connect(struct sock *ssk)
  *
  * Called when a SYN packet creates a new logical connection, i.e.
  * is not a join request.
+ *
+ * Return: true on success.
  */
-void mptcp_token_accept(struct mptcp_subflow_request_sock *req,
+bool mptcp_token_accept(struct mptcp_subflow_request_sock *req,
 			struct mptcp_sock *msk)
 {
 	struct mptcp_subflow_request_sock *pos;
 	struct sock *sk = (struct sock *)msk;
 	struct token_bucket *bucket;
+	bool ret = false;
 
-	sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
 	bucket = token_bucket(req->token);
 	spin_lock_bh(&bucket->lock);
+	if (hlist_nulls_unhashed(&req->token_node))
+		goto unlock;
 
-	/* pedantic lookup check for the moved token */
 	pos = __token_lookup_req(bucket, req->token);
-	if (!WARN_ON_ONCE(pos != req))
-		hlist_nulls_del_init_rcu(&req->token_node);
+	if (pos != req)
+		goto unlock;
+
+	hlist_nulls_del_init_rcu(&req->token_node);
 	__sk_nulls_add_node_rcu((struct sock *)msk, &bucket->msk_chain);
+	ret = true;
+
+unlock:
 	spin_unlock_bh(&bucket->lock);
+	if (ret)
+		sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
+
+	return ret;
 }
 
 bool mptcp_token_exists(u32 token)
@@ -355,16 +397,21 @@ void mptcp_token_destroy_request(struct request_sock *req)
 	struct mptcp_subflow_request_sock *pos;
 	struct token_bucket *bucket;
 
-	if (hlist_nulls_unhashed(&subflow_req->token_node))
+	if (hlist_nulls_unhashed_lockless(&subflow_req->token_node))
 		return;
 
 	bucket = token_bucket(subflow_req->token);
 	spin_lock_bh(&bucket->lock);
+	if (hlist_nulls_unhashed(&subflow_req->token_node))
+		goto unlock;
+
 	pos = __token_lookup_req(bucket, subflow_req->token);
-	if (!WARN_ON_ONCE(pos != subflow_req)) {
+	if (pos == subflow_req) {
 		hlist_nulls_del_init_rcu(&pos->token_node);
 		bucket->chain_len--;
 	}
+
+unlock:
 	spin_unlock_bh(&bucket->lock);
 }
 
diff --git a/net/mptcp/token_test.c b/net/mptcp/token_test.c
index 4fc39fa2e262..be9acce8a567 100644
--- a/net/mptcp/token_test.c
+++ b/net/mptcp/token_test.c
@@ -99,7 +99,7 @@ static void mptcp_token_test_accept(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, 0,
 			mptcp_token_new_request((struct request_sock *)req));
 	msk->token = req->token;
-	mptcp_token_accept(req, msk);
+	KUNIT_EXPECT_TRUE(test, mptcp_token_accept(req, msk));
 	KUNIT_EXPECT_PTR_EQ(test, msk, mptcp_token_get_sock(&init_net, msk->token));
 
 	/* this is now a no-op */
@@ -122,7 +122,7 @@ static void mptcp_token_test_destroyed(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, 0,
 			mptcp_token_new_request((struct request_sock *)req));
 	msk->token = req->token;
-	mptcp_token_accept(req, msk);
+	KUNIT_EXPECT_TRUE(test, mptcp_token_accept(req, msk));
 
 	/* simulate race on removal */
 	refcount_set(&sk->sk_refcnt, 0);
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net v3 0/2] mptcp: fix request migration ownership
  2026-08-06 11:14 [PATCH net v3 0/2] mptcp: fix request migration ownership Ren Wei
  2026-08-06 11:14 ` [PATCH net v3 1/2] mptcp: hold MP_JOIN msk ref when cloning reqsk Ren Wei
  2026-08-06 11:14 ` [PATCH net v3 2/2] mptcp: fix MP_CAPABLE token migration " Ren Wei
@ 2026-08-06 12:43 ` Matthieu Baerts
  2026-08-07  8:05   ` Nebula Security
  2 siblings, 1 reply; 5+ messages in thread
From: Matthieu Baerts @ 2026-08-06 12:43 UTC (permalink / raw)
  To: Ren Wei, netdev, mptcp
  Cc: martineau, geliang, davem, edumazet, kuba, pabeni, horms,
	ncardwell, kuniyu, daniel, kafai, kylebot, david.lee, vega,
	caoruide123, sashiko-bot

Hi Ren, Ruide,

Thank you for the v3! Here is a quick reaction, only about the
reproducers, not about the kernel code:

On 06/08/2026 13:14, Ren Wei wrote:

(...)

> Changes in v3:

(...)

> - Added a packetdrill MP_CAPABLE reproducer and decoded warning.

Nice, very appreciated, thank you!

I confirm they work well!
> // poc for MP_JOIN:
> 
> // Minimal reproducer for a stale subflow_req->msk after reqsk migration.
> --tolerance_usecs=200000
> --non_fatal=packet
> 
> `sysctl -q net.mptcp.enabled=1

Small details to help you next time to look at packetdrill reproducers
for MPTCP:

 - You can add new tests in gtests/net/mptcp/<dir>/<file>.pkt from the
   MPTCP packetdrill repository.
 - Then here above, you can add: `../common/defaults.sh`.
 - And execute your tests with run_all.py from the gtests/net dir, e.g.:
   ./packetdrill/run_all.py -lv4 mptcp/<dir>/<file>.pkt

These tests will then be executed from a dedicated netns, using many
sysctl settings: this helps to reproduce issues in a more controlled
environment.

> sysctl -q net.ipv4.tcp_migrate_req=1
> sysctl -q net.ipv4.tcp_synack_retries=1`

(detail) Here, tcp_synack_retries can be set to 0, then the sleep at the
end can be reduced.
> // Listener A and the owning MPTCP connection.
> +0     socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3
> +0     setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> +0     setsockopt(3, SOL_SOCKET, SO_REUSEPORT, [1], 4) = 0
> +0     bind(3, ..., ...) = 0
> +0     listen(3, 8) = 0
> 
> +0.0   <  addr[caddr0] > addr[saddr0]  S   0:0(0)         win 65535  <mss 1460, sackOK, TS val 1000 ecr 0,    nop, wscale 8, mpcapable v1 flags[flag_h] nokey>

Also for next time, when validating the listener side, feel free to drop
the TCP Timestamps option if it is not required (like what you did with
the other test): that makes the .pkt file more readable.

(...)

> MP_CAPABLE packetdrill reproducer:
> 
> // Reproducer for MP_CAPABLE request token ownership during TCP req migration.
> //
> // The first listener owns the request created by the MP_CAPABLE SYN.  A second
> // SO_REUSEPORT listener is added only after that SYN, then the first listener is
> // closed.  The SYN+ACK retransmission timer migrates the request to the second
> // listener, and a later request timer destroys the migrated request.
> //
> // On a vulnerable kernel, inet_reqsk_clone() raw-copies token_node.  The clone
> // is not the token table owner, so destroying the migrated request triggers the
> // MPTCP token ownership bug.
> --tolerance_usecs=250000
> 
> +0     `sysctl -q net.mptcp.enabled=1`
> +0     `sysctl -q net.ipv4.tcp_migrate_req=1`
> +0     `sysctl -q net.ipv4.tcp_synack_retries=2`
> +0     `sysctl -q net.ipv4.tcp_timestamps=1`
> +0     `sysctl -q kernel.panic_on_warn=0`
> +0     `sysctl -q kernel.panic_on_oops=0`
> +0     `ip tcp_metrics flush all >/dev/null 2>&1 || true`
> +0     `tc qdisc replace dev tun0 root pfifo >/dev/null 2>&1 || true`

Same here for `../common/defaults.sh`. Then I guess you only need
tcp_migrate_req=1 and tcp_synack_retries=2 (set to 0 to reduce the last
wait), and "wscale" will always be 8 in the S(.) instead of depending on
the environment.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v3 0/2] mptcp: fix request migration ownership
  2026-08-06 12:43 ` [PATCH net v3 0/2] mptcp: fix request migration ownership Matthieu Baerts
@ 2026-08-07  8:05   ` Nebula Security
  0 siblings, 0 replies; 5+ messages in thread
From: Nebula Security @ 2026-08-07  8:05 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: Ren Wei, netdev, mptcp, martineau, geliang, davem, edumazet, kuba,
	pabeni, horms, ncardwell, kuniyu, daniel, kafai, kylebot,
	david.lee, caoruide123, sashiko-bot

On Thu, Aug 6, 2026 at 5:43 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi Ren, Ruide,
>
> Thank you for the v3! Here is a quick reaction, only about the
> reproducers, not about the kernel code:
>
> On 06/08/2026 13:14, Ren Wei wrote:
>
> (...)
>
> > Changes in v3:
>
> (...)
>
> > - Added a packetdrill MP_CAPABLE reproducer and decoded warning.
>
> Nice, very appreciated, thank you!
>
> I confirm they work well!
> > // poc for MP_JOIN:
> >
> > // Minimal reproducer for a stale subflow_req->msk after reqsk migration.
> > --tolerance_usecs=200000
> > --non_fatal=packet
> >
> > `sysctl -q net.mptcp.enabled=1
>
> Small details to help you next time to look at packetdrill reproducers
> for MPTCP:
>
>  - You can add new tests in gtests/net/mptcp/<dir>/<file>.pkt from the
>    MPTCP packetdrill repository.
>  - Then here above, you can add: `../common/defaults.sh`.
>  - And execute your tests with run_all.py from the gtests/net dir, e.g.:
>    ./packetdrill/run_all.py -lv4 mptcp/<dir>/<file>.pkt
>
> These tests will then be executed from a dedicated netns, using many
> sysctl settings: this helps to reproduce issues in a more controlled
> environment.
>
> > sysctl -q net.ipv4.tcp_migrate_req=1
> > sysctl -q net.ipv4.tcp_synack_retries=1`
>
> (detail) Here, tcp_synack_retries can be set to 0, then the sleep at the
> end can be reduced.
> > // Listener A and the owning MPTCP connection.
> > +0     socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3
> > +0     setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> > +0     setsockopt(3, SOL_SOCKET, SO_REUSEPORT, [1], 4) = 0
> > +0     bind(3, ..., ...) = 0
> > +0     listen(3, 8) = 0
> >
> > +0.0   <  addr[caddr0] > addr[saddr0]  S   0:0(0)         win 65535  <mss 1460, sackOK, TS val 1000 ecr 0,    nop, wscale 8, mpcapable v1 flags[flag_h] nokey>
>
> Also for next time, when validating the listener side, feel free to drop
> the TCP Timestamps option if it is not required (like what you did with
> the other test): that makes the .pkt file more readable.
>
> (...)
>
> > MP_CAPABLE packetdrill reproducer:
> >
> > // Reproducer for MP_CAPABLE request token ownership during TCP req migration.
> > //
> > // The first listener owns the request created by the MP_CAPABLE SYN.  A second
> > // SO_REUSEPORT listener is added only after that SYN, then the first listener is
> > // closed.  The SYN+ACK retransmission timer migrates the request to the second
> > // listener, and a later request timer destroys the migrated request.
> > //
> > // On a vulnerable kernel, inet_reqsk_clone() raw-copies token_node.  The clone
> > // is not the token table owner, so destroying the migrated request triggers the
> > // MPTCP token ownership bug.
> > --tolerance_usecs=250000
> >
> > +0     `sysctl -q net.mptcp.enabled=1`
> > +0     `sysctl -q net.ipv4.tcp_migrate_req=1`
> > +0     `sysctl -q net.ipv4.tcp_synack_retries=2`
> > +0     `sysctl -q net.ipv4.tcp_timestamps=1`
> > +0     `sysctl -q kernel.panic_on_warn=0`
> > +0     `sysctl -q kernel.panic_on_oops=0`
> > +0     `ip tcp_metrics flush all >/dev/null 2>&1 || true`
> > +0     `tc qdisc replace dev tun0 root pfifo >/dev/null 2>&1 || true`
>
> Same here for `../common/defaults.sh`. Then I guess you only need
> tcp_migrate_req=1 and tcp_synack_retries=2 (set to 0 to reduce the last
> wait), and "wscale" will always be 8 in the S(.) instead of depending on
> the environment.
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>

Thanks so much for the detailed suggestions! We learned a lot from
them, and we’ll definitely take them into account for our future
patches.

We’ve also tried using packetdrill as the PoC for patches in other
parts of the networking stack, but some cases don’t seem to be fully
expressible with packetdrill. Anyway, we’ll try our best to use it
whenever possible.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-07  8:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 11:14 [PATCH net v3 0/2] mptcp: fix request migration ownership Ren Wei
2026-08-06 11:14 ` [PATCH net v3 1/2] mptcp: hold MP_JOIN msk ref when cloning reqsk Ren Wei
2026-08-06 11:14 ` [PATCH net v3 2/2] mptcp: fix MP_CAPABLE token migration " Ren Wei
2026-08-06 12:43 ` [PATCH net v3 0/2] mptcp: fix request migration ownership Matthieu Baerts
2026-08-07  8:05   ` Nebula Security

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox