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

Good catch — request_bkup and thmac are indeed read uninitialized in
subflow_ulp_clone() on the syncookie path (not stored in join_entry, not
restored). I've expanded the fix (v2) to save and restore both, completing the
state restore alongside local_id. v2 also updates the commit message
accordingly.

On Wed, Aug 12, 2026 at 3:38 AM <[email protected]> wrote:
>
> 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 <[email protected]>
>
> 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/[email protected]?part=1