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
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/31541882498
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/015cdc9962ec
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1144343
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)
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Medium] The `request_bkup` and `thmac` fields in `mptcp_subflow_request_sock` remain uninitialized when processing MP_JOIN SYN cookies, leading to state corruption and KMSAN violations.
--
--- Patch [1]: [PATCH mptcp] mptcp: fix uninitialized local_id in syncookie MP_JOIN reconstruction ---
commit 0b8fe993e0c1f5676def1ed9e644c11a85aca0ae
Author: Harshit Varu <harshitvaru666@gmail.com>
mptcp: fix uninitialized local_id in syncookie MP_JOIN reconstruction
This commit fixes an issue in MP_JOIN SYN cookie processing where the
local_id was not restored from the saved cookie entry. By restoring
subflow_req->local_id, it prevents the use of stale memory from previously
freed request sockets.
> 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: Medium]
This is a pre-existing issue, but do we also need to initialize request_bkup
and thmac here to prevent similar stale memory leaks?
It looks like these fields are not stored in the join_entry struct during
mptcp_join_store_state(), and therefore remain uninitialized in
mptcp_token_join_cookie_init_state().
Since the request socket slab is SLAB_TYPESAFE_BY_RCU and not zeroed, does
this cause uninitialized stale data to be copied into the new subflow context
when the child socket is cloned?
net/mptcp/subflow.c:subflow_ulp_clone() {
...
new_ctx->backup = subflow_req->backup;
new_ctx->request_bkup = subflow_req->request_bkup;
WRITE_ONCE(new_ctx->remote_id, subflow_req->remote_id);
new_ctx->token = subflow_req->token;
new_ctx->thmac = subflow_req->thmac;
...
}
Could this lead to state corruption for the path priority or uninitialized
memory access warnings if those fields are accessed later during packet
scheduling?
> subflow_req->token = e->token;
> subflow_req->msk = msk;
> spin_unlock_bh(&join_entry_locks[i]);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260811215135.87756-1-harshitvaru666@gmail.com?part=1
mptcp_token_join_cookie_init_state() rebuilds the request socket for a
MP_JOIN 4th-ACK that was handled under SYN cookies, but it only restores
remote_nonce, local_nonce, backup, join_id, token and msk from the saved
cookie entry. local_id, request_bkup and thmac are never restored, even
though the SYN path saves local_id and computes the other two.
subflow_ulp_clone() then reads those three fields and copies them into the
joined subflow context (local_id, request_bkup, thmac). Because the
request-sock slab is SLAB_TYPESAFE_BY_RCU and not zeroed on allocation, the
values are stale bytes of previously freed request sockets, which an
off-path peer can influence by sending concurrent MP_JOIN SYNs. A corrupted
local_id breaks id-based path-manager bookkeeping, and a corrupted
request_bkup misclassifies the subflow in the packet scheduler's
backup/active selection.
Save and restore request_bkup and thmac as well, completing the state
restore.
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 | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/mptcp/syncookies.c b/net/mptcp/syncookies.c
index 7f2252634..3c25ff627 100644
--- a/net/mptcp/syncookies.c
+++ b/net/mptcp/syncookies.c
@@ -27,7 +27,9 @@ struct join_entry {
u8 join_id;
u8 local_id;
u8 backup;
+ u8 request_bkup;
u8 valid;
+ u64 thmac;
};
#define COOKIE_JOIN_SLOTS 1024
@@ -63,8 +65,10 @@ static void mptcp_join_store_state(struct join_entry *entry,
entry->remote_nonce = subflow_req->remote_nonce;
entry->local_nonce = subflow_req->local_nonce;
entry->backup = subflow_req->backup;
+ entry->request_bkup = subflow_req->request_bkup;
entry->join_id = subflow_req->remote_id;
entry->local_id = subflow_req->local_id;
+ entry->thmac = subflow_req->thmac;
entry->valid = 1;
}
@@ -117,8 +121,11 @@ bool mptcp_token_join_cookie_init_state(struct mptcp_subflow_request_sock *subfl
subflow_req->remote_nonce = e->remote_nonce;
subflow_req->local_nonce = e->local_nonce;
subflow_req->backup = e->backup;
+ subflow_req->request_bkup = e->request_bkup;
subflow_req->remote_id = e->join_id;
+ subflow_req->local_id = e->local_id;
subflow_req->token = e->token;
+ subflow_req->thmac = e->thmac;
subflow_req->msk = msk;
spin_unlock_bh(&join_entry_locks[i]);
return true;
--
2.53.0
On 8/12/26 12:23 AM, Harshit Varu wrote:
> mptcp_token_join_cookie_init_state() rebuilds the request socket for a
> MP_JOIN 4th-ACK that was handled under SYN cookies, but it only restores
> remote_nonce, local_nonce, backup, join_id, token and msk from the saved
> cookie entry. local_id, request_bkup and thmac are never restored, even
> though the SYN path saves local_id and computes the other two.
>
> subflow_ulp_clone() then reads those three fields and copies them into the
> joined subflow context (local_id, request_bkup, thmac). Because the
> request-sock slab is SLAB_TYPESAFE_BY_RCU and not zeroed on allocation, the
> values are stale bytes of previously freed request sockets, which an
> off-path peer can influence by sending concurrent MP_JOIN SYNs. A corrupted
> local_id breaks id-based path-manager bookkeeping, and a corrupted
> request_bkup misclassifies the subflow in the packet scheduler's
> backup/active selection.
>
> Save and restore request_bkup and thmac as well, completing the state
> restore.
>
> 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 | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/mptcp/syncookies.c b/net/mptcp/syncookies.c
> index 7f2252634..3c25ff627 100644
> --- a/net/mptcp/syncookies.c
> +++ b/net/mptcp/syncookies.c
> @@ -27,7 +27,9 @@ struct join_entry {
> u8 join_id;
> u8 local_id;
> u8 backup;
> + u8 request_bkup;
> u8 valid;
> + u64 thmac;
Is thmac used for passive flows after
mptcp_token_join_cookie_init_state()? I think it's not. If so just init
to 0 in mptcp_token_join_cookie_init_state (with a comment) and remove
remove the new field from here.
Same for request_bkup, AFAICS both are used in syn_ack only.
The bottom line is that we don't want to increase `struct join_entry`
size without good reasons.
/P
Hi Harshit, Paolo,
@Paolo: thank you for the review!
On 12/08/2026 11:10, Paolo Abeni wrote:
> On 8/12/26 12:23 AM, Harshit Varu wrote:
>> mptcp_token_join_cookie_init_state() rebuilds the request socket for a
>> MP_JOIN 4th-ACK that was handled under SYN cookies, but it only restores
>> remote_nonce, local_nonce, backup, join_id, token and msk from the saved
>> cookie entry. local_id, request_bkup and thmac are never restored, even
>> though the SYN path saves local_id and computes the other two.
>>
>> subflow_ulp_clone() then reads those three fields and copies them into the
>> joined subflow context (local_id, request_bkup, thmac). Because the
>> request-sock slab is SLAB_TYPESAFE_BY_RCU and not zeroed on allocation, the
>> values are stale bytes of previously freed request sockets, which an
>> off-path peer can influence by sending concurrent MP_JOIN SYNs. A corrupted
>> local_id breaks id-based path-manager bookkeeping, and a corrupted
>> request_bkup misclassifies the subflow in the packet scheduler's
>> backup/active selection.
>>
>> Save and restore request_bkup and thmac as well, completing the state
>> restore.
>>
>> 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 | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/net/mptcp/syncookies.c b/net/mptcp/syncookies.c
>> index 7f2252634..3c25ff627 100644
>> --- a/net/mptcp/syncookies.c
>> +++ b/net/mptcp/syncookies.c
>> @@ -27,7 +27,9 @@ struct join_entry {
>> u8 join_id;
>> u8 local_id;
>> u8 backup;
>> + u8 request_bkup;
>> u8 valid;
>> + u64 thmac;
>
> Is thmac used for passive flows after
> mptcp_token_join_cookie_init_state()? I think it's not. If so just init
> to 0 in mptcp_token_join_cookie_init_state (with a comment) and remove
> remove the new field from here.
>
> Same for request_bkup, AFAICS both are used in syn_ack only.
>
> The bottom line is that we don't want to increase `struct join_entry`
> size without good reasons.
@Harshit: I agree with Paolo. So at the end, the v1 was good.
Can you please send a new version with what you had in the v1? Before
you do so, I have a few requests:
- Do not send a new version as a reply to another [1]
- Use 'PATCH net' [1]
- Wait 24h between submissions [1]
- Remove 'security@k.o' from Cc as it is sent to public MLs [2]
- Use ./scripts/get_maintainer.pl to cc the right people [3]
[1] https://docs.kernel.org/process/maintainer-netdev.html
[2] https://docs.kernel.org/process/submitting-patches.html
[3]
https://netdev-ctrl.bots.linux.dev/logs/build/1144354/14744385/cc_maintainers/desc
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
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/31544579623
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/c6dbff20aae1
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1144353
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)
© 2016 - 2026 Red Hat, Inc.