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

Paolo Abeni <[email protected]>
Newsgroups dev.linux.lists.mptcp,org.kernel.vger.netdev,org.kernel.vger.stable
Message-ID <[email protected]>
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: [email protected]
> Assisted-by: opencode:deepseek-v4-flash
> Signed-off-by: Harshit Varu <[email protected]>
> ---
>  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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.