Re: [PATCH net] mptcp: hold msk reference when cloning request sockets

Hi Yuan,

Thank you for your reply.

On 05/08/2026 10:52, Yuan Tan wrote:
> On Tue, Aug 4, 2026 at 11:00 AM Matthieu Baerts <[email protected]> wrote:
>>
>> Hi David, Kyle,
>>
>> (+cc Vega's people)
>>
>> On 04/08/2026 11:50, David Lee wrote:
>>> From: Kyle Zeng <[email protected]>
>>>
>>> An MP_JOIN request owns the reference stored in subflow_req->msk.
>>> inet_reqsk_clone() byte-copies that pointer when migrating a request,
>>> but does not acquire a reference for the clone.  The original and cloned
>>> request destructors can consequently drop the same reference, leaving
>>> one request with a dangling msk pointer.
>>>
>>> Let cloned MPTCP requests take their own msk reference.  The source
>>> request still owns its reference while it is being cloned, so sock_hold()
>>> is safe.  The clone's normal destructor balances the new reference on
>>> both successful and failed migration paths.
>>>
>>> Fixes: c905dee62232 ("tcp: Migrate TCP_NEW_SYN_RECV requests at retransmitting SYN+ACKs.")
>>
>> Thank you for this patch. It looks like it is similar to this one sent a
>> few months ago, but where changes have been requested:
>>
>>   https://lore.kernel.org/40fd38e7a368e5b7bc9bc83364a32241f977d53f.1778404619.git.caoruide123@gmail.com
>>
>> Do you mind checking what they did, and explaining the different approach, please?
>>
>> It seems there are two issues the Vega's team tried to solve: one with
>> MP_JOIN requests (what you are trying to fix here) and with MP_CAPABLE.
>> For me, it is fine to split that in two patches. I also don't mind who
>> is writing the final patch(es), as long as credits are given. If you,
>> David/Kyle, are doing that, it might be OK to add a:
>>
>>   Reported-by: Vega <[email protected]>
>>   Closes: https://lore.kernel.org/40fd38e7a368e5b7bc9bc83364a32241f977d53f.1778404619.git.caoruide123@gmail.com
>>
>> @Vega's team: OK with that? Or are you actively working on a v3?
> 
> We seem to have lost track of this patch, and we sincerely apologize.
> 
> Our patch addresses two issues: one involving MP_JOIN requests and the
> other involving MP_CAPABLE.
> 
> I reviewed Kyle and David’s fix for the MP_JOIN issue, and it looks
> reasonable to me.

Please note that Clashiko reported an issue. A new version *might* be
required:


https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260804095051.715355-1-david.lee%40trailofbits.com

Note that both Clashiko and Sashiko are mentioning the other existing
issue with MP_CAPABLE, that the Vega's team tried to fix in their v2:


https://sashiko.dev/#/patchset/20260804095051.715355-1-david.lee%40trailofbits.com

I think the fix can be done in two parts.

> Ruide and I also do not have a preference regarding whose patch is accepted.
> 
> If David/Kyle' patch is accepted, please add:
> 
> Reported-by: Vega <[email protected]>
> Reported-by: Ruide Cao <[email protected]>
> Closes: https://lore.kernel.org/40fd38e7a368e5b7bc9bc83364a32241f977d53f.1778404619.git.caoruide123@gmail.com
If a new version is required, please sync on who is doing what. Let's
wait for Kyle and David's reply. (In case of "timeout" (after a few
days), feel free to work on the v3.)

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