[PATCH mptcp-net] mptcp: disconnect: reset first subflow's local_id

[PATCH mptcp-net] mptcp: disconnect: reset first subflow's local_id

[Matthieu Baerts (NGI0)]

The first subflow can get disconnected for various reasons and
mptcp_subflow_ctx_reset() will reset most fields and set local_id to -1.
That's fine until the subflow is reused after a reconnect.

As mentioned by Clashiko [1]: on the retry, local_id is -1, so the
connect goes through:

  tcp_connect()
    subflow_rebuild_header()
      subflow_chk_local_id()
        mptcp_pm_get_local_id()

mptcp_pm_get_local_id() returns 0 only when the msk local address
matches the subflow's address. The addresses can change, but even if
they don't, for an unbound msk, the address is therefore still zero at
this point. (Note: in MPTCP disconnect selftests, the address is the
same, and mptcp_pm_get_local_id() will return 0.)

At this stage, the kernel PM might then give the MPC subflow a non-zero
ID, either from a matching endpoint or by adding an "implicit" endpoint.
The RFC 8684 says that the first subflow always has the ID 0, and the
PMs rely on that for special cases. So it is important to reset it to 0.

Note that we need to keep it to -1 in mptcp_subflow_ctx_reset() to avoid
a double closures in mptcp_close_ssk().

This issue was found by code analysis, and confirmed by looking at the
ID after a reconnection.

Fixes: a7cfe7766370 ("mptcp: fix data races on local_id")
Link: https://lore.kernel.org/179198834484.434549.4616694240552704738@kernel.org
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/protocol.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 60cbd91be632..4a6f16e0d809 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3654,6 +3654,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
 	 */
 	mptcp_do_fastclose(sk);
 	mptcp_destroy_common(msk);
+	WRITE_ONCE(mptcp_subflow_ctx(msk->first)->local_id, 0);
 
 	/* The first subflow is already in TCP_CLOSE status, the following
 	 * can't overlap with a fallback anymore

---
base-commit: 6a6728a8e881c12c984d104403ecaa0523d3b7e8
change-id: 20261015-mptcp-first-disco-id-reset-a0866d67d9a6

Best regards,
--  
Matthieu Baerts (NGI0) <matttbe@kernel.org>

Re: [PATCH mptcp-net] mptcp: disconnect: reset first subflow's local_id

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] mptcp: NULL pointer dereference in mptcp_disconnect() on unconnected socket
--

commit 79fba9c250462a2f6852b46cf0562a508bcae604
Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>

mptcp: disconnect: reset first subflow's local_id

This commit resets the local_id of the first subflow to 0 when disconnecting.
This ensures the value is correctly initialized if the subflow is reused
after a reconnect.

Link: https://lore.kernel.org/179198834484.434549.4616694240552704738@kernel.org

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3654,6 +3654,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
>  	 */
>  	mptcp_do_fastclose(sk);
>  	mptcp_destroy_common(msk);
> +	WRITE_ONCE(mptcp_subflow_ctx(msk->first)->local_id, 0);

[Severity: Critical]
Does this code cause a NULL pointer dereference in mptcp_disconnect() on an
unconnected socket?

If a user creates an MPTCP socket via socket(AF_MPTCP, SOCK_STREAM, 0),
msk->first is initially NULL. If the user then calls connect() with
AF_UNSPEC before any connection is made, the networking core handles
AF_UNSPEC by directly invoking the socket's disconnect handler,
mptcp_disconnect().

At this point, msk->first is still NULL. Passing it to mptcp_subflow_ctx()
yields an invalid pointer, leading to an immediate page fault when local_id
is written.

>  
>  	/* The first subflow is already in TCP_CLOSE status, the following
>  	 * can't overlap with a fallback anymore

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-mptcp-first-disco-id-reset-v1-1-6ae41e8f02c6@kernel.org?part=1

Re: [PATCH mptcp-net] mptcp: disconnect: reset first subflow's local_id

[Matthieu Baerts]

On 15/10/2026 16:05, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] mptcp: NULL pointer dereference in mptcp_disconnect() on unconnected socket
> --
> 
> commit 79fba9c250462a2f6852b46cf0562a508bcae604
> Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> mptcp: disconnect: reset first subflow's local_id
> 
> This commit resets the local_id of the first subflow to 0 when disconnecting.
> This ensures the value is correctly initialized if the subflow is reused
> after a reconnect.
> 
> Link: https://lore.kernel.org/179198834484.434549.4616694240552704738@kernel.org
> 
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -3654,6 +3654,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
>>  	 */
>>  	mptcp_do_fastclose(sk);
>>  	mptcp_destroy_common(msk);
>> +	WRITE_ONCE(mptcp_subflow_ctx(msk->first)->local_id, 0);
> 
> [Severity: Critical]
> Does this code cause a NULL pointer dereference in mptcp_disconnect() on an
> unconnected socket?
> 
> If a user creates an MPTCP socket via socket(AF_MPTCP, SOCK_STREAM, 0),

What? AF_MPTCP???

> msk->first is initially NULL. If the user then calls connect() with
> AF_UNSPEC before any connection is made, the networking core handles
> AF_UNSPEC by directly invoking the socket's disconnect handler,
> mptcp_disconnect().

Arf, I forgot about this weird path... I checked with shutdown() (which
is not allowed for unconnected sockets). I will add a check and add a
packetdrill test for this.

> At this point, msk->first is still NULL. Passing it to mptcp_subflow_ctx()
> yields an invalid pointer, leading to an immediate page fault when local_id
> is written.
> 
>>  
>>  	/* The first subflow is already in TCP_CLOSE status, the following
>>  	 * can't overlap with a fallback anymore
> 

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-net] mptcp: disconnect: reset first subflow's local_id

[MPTCP CI]

Hi Matthieu,

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): Unstable: 1 failed test(s): selftest_diag ⚠️ 
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/38750500938

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


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)