MPTCP Linux Development
 help / color / mirror / Atom feed
* [PATCH mptcp-next v3] mptcp: fix skb_ext leak in fallback mode
@ 2026-09-02  8:39 Geliang Tang
  2026-09-02  9:55 ` MPTCP CI
  2026-09-02 10:20 ` Matthieu Baerts
  0 siblings, 2 replies; 4+ messages in thread
From: Geliang Tang @ 2026-09-02  8:39 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

In fallback mode, MPTCP sockets behave as plain TCP and should not allocate
SKB_EXT_MPTCP for transmitted skbs. The current code unconditionally
allocates the extension, causing memory leaks when skbs are freed without
releasing it.

Fix by short-circuiting __mptcp_add_ext() in fallback mode and skipping all
DSS bookkeeping in mptcp_sendmsg_frag(). Also allow TCP coalescing when
mpext is NULL in fallback mode.

This latent bug will be exposed once TLS ULP support is added to fallback
MPTCP sockets, as each sendmsg via the TLS path would leak one skb_ext
object.

Fixes: 3a54a74a3c5b ("mptcp: allocate TX skbs in msk context")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
v3:
 - Free extensions before early returns (fixes memory leak)
 - Use fallback label to skip mpext operations (fixes NULL deref)
 - Allow TCP coalescing when mpext NULL in fallback mode
 - Remove cached fb variable to avoid race conditions (fixes NULL deref)
 - Pass fallback state to mptcp_skb_can_collapse_to() for correct behavior

v2:
 - Free extensions before early returns
 - Added fallback label to skip mpext operations
 - Allow TCP coalescing when mpext NULL
 - Cache fallback state in bool fb
 - https://patchwork.kernel.org/project/mptcp/patch/b67dec47d321886d45fdd2bca1c303314fcdb229.1788252583.git.tanggeliang@kylinos.cn/

v1:
 - https://patchwork.kernel.org/project/mptcp/patch/70a7e7e05337faa0547c3759e5d9829763f2bcc5.1788244452.git.tanggeliang@kylinos.cn/
---
 net/mptcp/protocol.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 0b24e0afedfb..7338dc3b70eb 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1153,15 +1153,20 @@ bool mptcp_schedule_work(struct sock *sk)
 
 static bool mptcp_skb_can_collapse_to(u64 write_seq,
 				      const struct sk_buff *skb,
-				      const struct mptcp_ext *mpext)
+				      const struct mptcp_ext *mpext,
+				      bool fallback)
 {
 	if (!tcp_skb_can_collapse_to(skb))
 		return false;
 
+	/* In fallback mode, allow coalescing into extension-less SKBs */
+	if (!mpext)
+		return fallback;
+
 	/* can collapse only if MPTCP level sequence is in order and this
 	 * mapping has not been xmitted yet
 	 */
-	return mpext && mpext->data_seq + mpext->data_len == write_seq &&
+	return mpext->data_seq + mpext->data_len == write_seq &&
 	       !mpext->frozen;
 }
 
@@ -1361,7 +1366,8 @@ static struct sk_buff *__mptcp_do_alloc_tx_skb(struct sock *sk, gfp_t gfp)
 
 	skb = alloc_skb_fclone(MAX_TCP_HEADER, gfp);
 	if (likely(skb)) {
-		if (likely(__mptcp_add_ext(skb, gfp))) {
+		if (unlikely(__mptcp_check_fallback(mptcp_sk(sk))) ||
+		    likely(__mptcp_add_ext(skb, gfp))) {
 			skb_reserve(skb, MAX_TCP_HEADER);
 			skb->ip_summed = CHECKSUM_PARTIAL;
 			INIT_LIST_HEAD(&skb->tcp_tsorted_anchor);
@@ -1471,7 +1477,8 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 		 * SSN association set here
 		 */
 		mpext = mptcp_get_ext(skb);
-		if (!mptcp_skb_can_collapse_to(data_seq, skb, mpext)) {
+		if (!mptcp_skb_can_collapse_to(data_seq, skb, mpext,
+					       __mptcp_check_fallback(msk))) {
 			TCP_SKB_CB(skb)->eor = 1;
 			tcp_mark_push(tcp_sk(ssk), skb);
 			goto alloc_skb;
@@ -1507,6 +1514,8 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 		 */
 		if (snd_una != msk->snd_nxt || skb->len ||
 		    skb != tcp_send_head(ssk)) {
+			if (unlikely(__mptcp_check_fallback(msk)) && mpext)
+				skb_ext_del(skb, SKB_EXT_MPTCP);
 			tcp_remove_empty_skb(ssk);
 			return 0;
 		}
@@ -1518,6 +1527,8 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 
 	copy = min_t(size_t, copy, info->limit - info->sent);
 	if (!sk_wmem_schedule(ssk, copy)) {
+		if (unlikely(__mptcp_check_fallback(msk)) && mpext)
+			skb_ext_del(skb, SKB_EXT_MPTCP);
 		tcp_remove_empty_skb(ssk);
 		return -ENOMEM;
 	}
@@ -1538,6 +1549,15 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 	TCP_SKB_CB(skb)->end_seq += copy;
 	tcp_skb_pcount_set(skb, 0);
 
+	/* in fallback mode, skip DSS bookkeeping and free the extension
+	 * if allocated
+	 */
+	if (unlikely(__mptcp_check_fallback(msk))) {
+		if (mpext)
+			skb_ext_del(skb, SKB_EXT_MPTCP);
+		goto fallback;
+	}
+
 	/* on skb reuse we just need to update the DSS len */
 	if (reuse_skb) {
 		TCP_SKB_CB(skb)->tcp_flags &= ~TCPHDR_PSH;
@@ -1571,6 +1591,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 	if (mptcp_subflow_ctx(ssk)->send_infinite_map)
 		mptcp_update_infinite_map(msk, ssk, mpext);
 	trace_mptcp_sendmsg_frag(mpext);
+fallback:
 	mptcp_subflow_ctx(ssk)->rel_write_seq += copy;
 
 	/* if this is the last chunk of a dfrag with MSG_EOR set,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH mptcp-next v3] mptcp: fix skb_ext leak in fallback mode
  2026-09-02  8:39 [PATCH mptcp-next v3] mptcp: fix skb_ext leak in fallback mode Geliang Tang
@ 2026-09-02  9:55 ` MPTCP CI
  2026-09-02 10:20 ` Matthieu Baerts
  1 sibling, 0 replies; 4+ messages in thread
From: MPTCP CI @ 2026-09-02  9:55 UTC (permalink / raw)
  To: Geliang Tang; +Cc: mptcp

Hi Geliang,

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): Success! ✅
- 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: 
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/33611802226

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


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)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH mptcp-next v3] mptcp: fix skb_ext leak in fallback mode
  2026-09-02  8:39 [PATCH mptcp-next v3] mptcp: fix skb_ext leak in fallback mode Geliang Tang
  2026-09-02  9:55 ` MPTCP CI
@ 2026-09-02 10:20 ` Matthieu Baerts
  2026-09-11 23:07   ` Geliang Tang
  1 sibling, 1 reply; 4+ messages in thread
From: Matthieu Baerts @ 2026-09-02 10:20 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 02/09/2026 10:39, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> In fallback mode, MPTCP sockets behave as plain TCP and should not allocate
> SKB_EXT_MPTCP for transmitted skbs. The current code unconditionally
> allocates the extension, causing memory leaks when skbs are freed without
> releasing it.

I'm surprised by this. Do you have kmemleak reports?

Or is it linked to this issue?

  https://github.com/multipath-tcp/mptcp_net-next/issues/583

I'm asking because I would think that such issues would have been caught
by kmemleak, plus the fix is quite invasive.

> Fix by short-circuiting __mptcp_add_ext() in fallback mode and skipping all
> DSS bookkeeping in mptcp_sendmsg_frag(). Also allow TCP coalescing when
> mpext is NULL in fallback mode.
> 
> This latent bug will be exposed once TLS ULP support is added to fallback
> MPTCP sockets, as each sendmsg via the TLS path would leak one skb_ext
> object.

Do you mean this bug is only visible with the new feature? If yes, then
no need to threat it as a fix.

Then, I'm also wondering if this patch shouldn't be specific to this
case: maybe less code should be modified to deal with fallback sockets
once this happens? (I'm probably missing context to really help here)

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH mptcp-next v3] mptcp: fix skb_ext leak in fallback mode
  2026-09-02 10:20 ` Matthieu Baerts
@ 2026-09-11 23:07   ` Geliang Tang
  0 siblings, 0 replies; 4+ messages in thread
From: Geliang Tang @ 2026-09-11 23:07 UTC (permalink / raw)
  To: Matthieu Baerts, mptcp; +Cc: Geliang Tang

Hi Matt,

Thanks for the review.

On Wed, 2026-09-02 at 12:20 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 02/09/2026 10:39, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > In fallback mode, MPTCP sockets behave as plain TCP and should not
> > allocate
> > SKB_EXT_MPTCP for transmitted skbs. The current code
> > unconditionally
> > allocates the extension, causing memory leaks when skbs are freed
> > without
> > releasing it.
> 
> I'm surprised by this. Do you have kmemleak reports?
> 
> Or is it linked to this issue?
> 
>   https://github.com/multipath-tcp/mptcp_net-next/issues/583
> 
> I'm asking because I would think that such issues would have been
> caught
> by kmemleak, plus the fix is quite invasive.

This is another kmemleak, which only occurred when I ran the fallback
TLS test after enabling the TLS settings on the fallback socket.

> 
> > Fix by short-circuiting __mptcp_add_ext() in fallback mode and
> > skipping all
> > DSS bookkeeping in mptcp_sendmsg_frag(). Also allow TCP coalescing
> > when
> > mpext is NULL in fallback mode.
> > 
> > This latent bug will be exposed once TLS ULP support is added to
> > fallback
> > MPTCP sockets, as each sendmsg via the TLS path would leak one
> > skb_ext
> > object.
> 
> Do you mean this bug is only visible with the new feature? If yes,
> then
> no need to threat it as a fix.

In v4, I removed the "Fixes" tag and changed it to for "mptcp-next".

> 
> Then, I'm also wondering if this patch shouldn't be specific to this
> case: maybe less code should be modified to deal with fallback

Yes, it's a version with less code in v4, and it fixes the root cause
of this issue.

Thanks,
-Geliang

> sockets
> once this happens? (I'm probably missing context to really help here)
> 
> Cheers,
> Matt

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-11 23:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02  8:39 [PATCH mptcp-next v3] mptcp: fix skb_ext leak in fallback mode Geliang Tang
2026-09-02  9:55 ` MPTCP CI
2026-09-02 10:20 ` Matthieu Baerts
2026-09-11 23:07   ` Geliang Tang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox