Re: [PATCH mptcp v2] mptcp: restore full join state in syncookie MP_JOIN reconstruction

[Matthieu Baerts <matttbe@kernel.org>]

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.