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

Yuan Tan <[email protected]>
Newsgroups org.kernel.vger.stable,dev.linux.lists.mptcp,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <CAPuPA7KZhv39w=nCG71WBLOcPRrrpb=CvDdVcnCRbfu+z9ZELg@mail.gmail.com>
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.

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

Btw, Kyle and David, would you be open to collaborating with us on
Linux kernel patches? Over the past few months, our team has fixed
more than 100 high-severity vulnerabilities in the kernel. We’ve
learned a great deal from maintainers’ feedback and have built up
extensive experience. We’d love to help improve kernel security while
minimizing the burden on maintainers as much as possible.

>
> One last thing: it looks like your patch is conflicting with this one:
>
>   https://lore.kernel.org/netdev/[email protected]/
>
> > Cc: [email protected]
> > Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
> > Signed-off-by: Kyle Zeng <[email protected]>
> > Co-developed-by: David Lee <[email protected]>
> > Signed-off-by: David Lee <[email protected]>
> > ---
> > Bug found and triaged by OpenAI Security Research and
> > validated by Trail of Bits.
> >
> > Trail of Bits has a reproducer for this bug that triggers a
> > KASAN use-after-free and can share if needed.
>
> I think it would be good to include the (decoded) KASAN warning in the
> commit message. Regarding the reproducer, if it is with packetdrill, I
> think it is always useful to share it. For others, don't hesitate to
> share it to the same people: it can help better understanding the issue.
> Maybe remove the mailing list when doing that if it is security related?
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>
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.