Re: [PATCH net v3 2/2] mptcp: fix MP_CAPABLE token migration when cloning reqsk

[Matthieu Baerts <matttbe@kernel.org>]

Hi Ren, Ruide,

On 06/08/2026 13:14, Ren Wei wrote:
> 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.

Thank you for the new version!

I have one major comment, please see below:
> 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);

Here, you cannot call tcp_done(nsk): nsk is an MPTCP socket, not a TCP
one. You probably want to destroy the msk instead.

(In theory, your reproducer launched on a kernel with your patches
applied on top of MPTCP's export branch [1] should produce a warning)

Apart from that, Clashiko has a few comments. Do you mind checking them
and reply to each comment here, before sending a new version, whether
you agree/disagree or if it is unclear, please?

[1] https://github.com/multipath-tcp/mptcp_net-next/

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
pw-bot: cr