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

[Matthieu Baerts <matttbe@kernel.org>]

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.