[PATCH net] mptcp: fix uninitialized local_id in syncookie MP_JOIN reconstruction

[PATCH net] mptcp: fix uninitialized local_id in syncookie MP_JOIN reconstruction

[Harshit Varu]

mptcp_token_join_cookie_init_state() restores remote_nonce, local_nonce,
backup, join_id, token and msk from the saved cookie entry when rebuilding
the request socket for a MP_JOIN 4th-ACK handled under SYN cookies, but it
does not restore local_id, even though the SYN path saved it.
subflow_ulp_clone() then reads that uninitialized field and stores it as
the joined subflow's address-ID. Because the request-sock slab is
SLAB_TYPESAFE_BY_RCU and not zeroed on allocation, the value is the stale
byte of a previously freed request socket, which an off-path peer can
influence by sending concurrent MP_JOIN SYNs. This corrupts the path
manager's id-based subflow bookkeeping for the connection.

Restore subflow_req->local_id from the cookie entry, as done for the other
fields.

Fixes: 9466a1ccebbe ("mptcp: enable JOIN requests even if cookies are in use")
Cc: stable@vger.kernel.org
Assisted-by: opencode:deepseek-v4-flash
Signed-off-by: Harshit Varu <harshitvaru666@gmail.com>
---
 net/mptcp/syncookies.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mptcp/syncookies.c b/net/mptcp/syncookies.c
index 7f2252634..b5cac5701 100644
--- a/net/mptcp/syncookies.c
+++ b/net/mptcp/syncookies.c
@@ -118,6 +118,7 @@ bool mptcp_token_join_cookie_init_state(struct mptcp_subflow_request_sock *subfl
 	subflow_req->local_nonce = e->local_nonce;
 	subflow_req->backup = e->backup;
 	subflow_req->remote_id = e->join_id;
+	subflow_req->local_id = e->local_id;
 	subflow_req->token = e->token;
 	subflow_req->msk = msk;
 	spin_unlock_bh(&join_entry_locks[i]);
-- 
2.53.0

Re: [PATCH net] mptcp: fix uninitialized local_id in syncookie MP_JOIN reconstruction

[MPTCP CI]

Hi Harshit,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/31884229084

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/6ac3c6cc38ea
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1146490


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH net] mptcp: fix uninitialized local_id in syncookie MP_JOIN reconstruction

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `request_bkup` bitfield in `mptcp_subflow_request_sock` is left uninitialized during syncookie MP_JOIN reconstruction, causing stale data to dictate the subflow backup state.
--

--- Patch [1]: [PATCH net] mptcp: fix uninitialized local_id in syncookie MP_JOIN reconstruction ---
commit ac471f0880c8ac59556dc5bd33e8a83b1849726c
Author: Harshit Varu <harshitvaru666@gmail.com>

mptcp: fix uninitialized local_id in syncookie MP_JOIN reconstruction

This commit restores the local_id field from the saved cookie entry when
rebuilding the request socket for an MP_JOIN 4th-ACK handled under SYN
cookies. This prevents the subflow from using stale slab memory as its
address-ID, protecting the path manager's bookkeeping.

> diff --git a/net/mptcp/syncookies.c b/net/mptcp/syncookies.c
> index 7f22526346a7e..b5cac57011221 100644
> --- a/net/mptcp/syncookies.c
> +++ b/net/mptcp/syncookies.c
> @@ -118,6 +118,7 @@ bool mptcp_token_join_cookie_init_state(struct mptcp_subflow_request_sock *subfl
>  	subflow_req->local_nonce = e->local_nonce;
>  	subflow_req->backup = e->backup;
>  	subflow_req->remote_id = e->join_id;
> +	subflow_req->local_id = e->local_id;

[Severity: High]
This is a pre-existing issue, but does this code also leave the request_bkup
bitfield uninitialized during syncookie reconstruction?

While this commit fixes local_id, the request_bkup bitfield in
mptcp_subflow_request_sock doesn't appear to be saved in
mptcp_join_store_state() or restored here.

Since the request_sock slab is SLAB_TYPESAFE_BY_RCU and memory isn't zeroed
on allocation, subflow_init_req() zeroes some adjacent bitfields but misses
request_bkup:

net/mptcp/subflow.c:subflow_init_req() {
    ...
    subflow_req->mp_capable = 0;
    subflow_req->mp_join = 0;
    subflow_req->csum_reqd = mptcp_is_checksum_enabled(sock_net(sk_listener));
    ...
}

When the final subflow context is cloned, it copies this uninitialized state
directly from the request socket:

net/mptcp/subflow.c:subflow_ulp_clone() {
    ...
    new_ctx->backup = subflow_req->backup;
    new_ctx->request_bkup = subflow_req->request_bkup;
    ...
}

Could this stale memory cause the MPTCP stack to erroneously treat primary
paths as backup paths (or vice versa)?

>  	subflow_req->token = e->token;
>  	subflow_req->msk = msk;
>  	spin_unlock_bh(&join_entry_locks[i]);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815115205.197151-1-harshitvaru666@gmail.com?part=1