Re: [PATCH net v6 0/2] mptcp: fix request migration ownership

[Matthieu Baerts <matttbe@kernel.org>]

Hi Ren, Ruide,

On 08/09/2026 18:41, Ren Wei wrote:
> TCP request migration clones pending requests with inet_reqsk_clone(). Some
> MPTCP request fields carry ownership which cannot be duplicated 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 acquire a reference while verifying that the original request still
> owns the same msk.
> 
> 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. Patch 2 moves token ownership under the bucket lock and makes token
> acceptance and destruction tolerate an already moved token.

Thank you for your patience. It looks like we were waiting for Clashiko
report, but it didn't get processed for some unknown reason, sorry about
that.

The patches look good to me, I suggest taking them in the MPTCP tree,
and I will send them later on.

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

(...)

> // poc for MP_JOIN:

Thank you for the PoCs using packetdrill! Do you mind creating a PR to
add them in the MPTCP fork, please:

  https://github.com/multipath-tcp/packetdrill/

(if it is an issue, please tell me, I can add them later)

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

Usually, 100000 is used. Do you need more?

> --non_fatal=packet

Why do you need this?
> `sysctl -q net.mptcp.enabled=1

This one is not needed, you can use '../common/defaults.sh' instead.

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

(When validating the listener side, you can drop the TCP TS options,
except if you do need them, clearer with less options)

> +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)

If such calltrace is on public ML, please, next time, include this in
the commit message instead (without the prefixes between []). I will
amend it in the commit message.

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

You probably only require tcp_migrate_req=1 here, after
../common/defaults.sh.

> +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]>

"wscale 9" is "strange". Note that you can launch tests with run_all.py,
e.g.

  # ./packetdrill/run_all.py -lv mptcp/<subdir>/<test>.pkt

> +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`

If you set `sysctl -q net.ipv4.tcp_synack_retries=0`, can you reduce
this to 2 seconds instead of 8?

> +0     close(4) = 0
> 
> ------------------------------
> decoded warning from MP_CAPABLE reproducer:

Same here, I will add it in the commit messages.

> [  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 ]---

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