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

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

[David Lee]

From: Kyle Zeng <kylebot@openai.com>

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.")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Co-developed-by: David Lee <david.lee@trailofbits.com>
Signed-off-by: David Lee <david.lee@trailofbits.com>
---
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.

 include/net/mptcp.h             |  5 +++++
 net/ipv4/inet_connection_sock.c | 10 ++++++++--
 net/mptcp/subflow.c             |  8 ++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 71b9fc5a5..51d4b02f5 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -223,6 +223,7 @@ int mptcp_subflow_init_cookie_req(struct request_sock *req,
 struct request_sock *mptcp_subflow_reqsk_alloc(const struct request_sock_ops *ops,
 					       struct sock *sk_listener,
 					       bool attach_listener);
+void mptcp_subflow_reqsk_clone(struct request_sock *req);
 
 __be32 mptcp_get_reset_option(const struct sk_buff *skb);
 
@@ -309,6 +310,10 @@ static inline struct request_sock *mptcp_subflow_reqsk_alloc(const struct reques
 	return NULL;
 }
 
+static inline void mptcp_subflow_reqsk_clone(struct request_sock *req)
+{
+}
+
 static inline __be32 mptcp_reset_option(const struct sk_buff *skb)  { return htonl(0u); }
 
 static inline void mptcp_active_detect_blackhole(struct sock *sk, bool expired) { }
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 56902bba5..f3ef9c15a 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -17,6 +17,7 @@
 #include <net/inet_timewait_sock.h>
 #include <net/ip.h>
 #include <net/route.h>
+#include <net/mptcp.h>
 #include <net/tcp_states.h>
 #include <net/xfrm.h>
 #include <net/tcp.h>
@@ -946,8 +947,13 @@ static struct request_sock *inet_reqsk_clone(struct request_sock *req,
 	/* We need not acquire fastopenq->lock
 	 * because the child socket is locked in inet_csk_listen_stop().
 	 */
-	if (sk->sk_protocol == IPPROTO_TCP && tcp_rsk(nreq)->tfo_listener)
-		rcu_assign_pointer(tcp_sk(nreq->sk)->fastopen_rsk, nreq);
+	if (sk->sk_protocol == IPPROTO_TCP) {
+		if (tcp_rsk(nreq)->tfo_listener)
+			rcu_assign_pointer(tcp_sk(nreq->sk)->fastopen_rsk, nreq);
+
+		if (rsk_is_mptcp(req))
+			mptcp_subflow_reqsk_clone(nreq);
+	}
 
 	return nreq;
 }
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 8e386899c..1b6aa5e8c 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -47,6 +47,14 @@ static void subflow_req_destructor(struct request_sock *req)
 	mptcp_token_destroy_request(req);
 }
 
+void mptcp_subflow_reqsk_clone(struct request_sock *req)
+{
+	struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
+
+	if (subflow_req->msk)
+		sock_hold((struct sock *)subflow_req->msk);
+}
+
 static void subflow_generate_hmac(u64 key1, u64 key2, u32 nonce1, u32 nonce2,
 				  void *hmac)
 {
-- 
2.47.3

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

[Matthieu Baerts]

Hi David, Kyle,

(+cc Vega's people)

On 04/08/2026 11:50, David Lee wrote:
> From: Kyle Zeng <kylebot@openai.com>
> 
> 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 <vega@nebusec.ai>
  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?

One last thing: it looks like your patch is conflicting with this one:

  https://lore.kernel.org/netdev/20260803061739.134737-1-jiayuan.chen@linux.dev/

> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> Co-developed-by: David Lee <david.lee@trailofbits.com>
> Signed-off-by: David Lee <david.lee@trailofbits.com>
> ---
> 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.

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

[Yuan Tan]

On Tue, Aug 4, 2026 at 11:00 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi David, Kyle,
>
> (+cc Vega's people)
>
> On 04/08/2026 11:50, David Lee wrote:
> > From: Kyle Zeng <kylebot@openai.com>
> >
> > 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 <vega@nebusec.ai>
>   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 <vega@nebusec.ai>
Reported-by: Ruide Cao <caoruide123@gmail.com>
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/20260803061739.134737-1-jiayuan.chen@linux.dev/
>
> > Cc: stable@vger.kernel.org
> > Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
> > Signed-off-by: Kyle Zeng <kylebot@openai.com>
> > Co-developed-by: David Lee <david.lee@trailofbits.com>
> > Signed-off-by: David Lee <david.lee@trailofbits.com>
> > ---
> > 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.
>

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

[Matthieu Baerts]

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 <matttbe@kernel.org> wrote:
>>
>> Hi David, Kyle,
>>
>> (+cc Vega's people)
>>
>> On 04/08/2026 11:50, David Lee wrote:
>>> From: Kyle Zeng <kylebot@openai.com>
>>>
>>> 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 <vega@nebusec.ai>
>>   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 <vega@nebusec.ai>
> Reported-by: Ruide Cao <caoruide123@gmail.com>
> 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.