[PATCH mptcp-next v3 0/3] mptcp: pm: fix reachable extra_subflows guards

[PATCH mptcp-next v3 0/3] mptcp: pm: fix reachable extra_subflows guards

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Hi,

Following up on issue #629, here is a fix series for the issues
reported against e99c1ca871 ("mptcp: pm: add WARN_ON_ONCE guards
on extra_subflows underflow").

Patch 1 bounds the userspace PM admission at U8_MAX, so the u8 can
no longer be wrapped by a peer establishing more than 255 subflows.

Patch 2 addresses the disconnect() vs MP_JOIN race: as discussed,
the decrement sites now check the msk state first and skip the
accounting when the msk is already in TCP_CLOSE, as the state is set
before mptcp_pm_data_reset() clears the counters, and once the msk
is closed the accounting is not relevant anymore. No new lock is
involved. The guards themselves are downgraded to a clamp with a
rate-limited pr_warn(), as the warn was reachable and turned into a
remotely triggerable panic on panic_on_warn kernels.

Patch 3, from Quanye, applies the same bound to the Netlink subflow
creation path.

Changes since v2:

- Patch 3: label moved inside the if-statement, a note about the
  pre-existing unconditional local address deletion in the reused
  error path, and the spurious empty line before the SoBs dropped

Changes since v1 [1]:

- Patch 1: re-use "ret" instead of a new variable, drop the
  comment, Co-developed-by added for Quanye, who sent the same fix
  independently
- issue reference: Link: on patches 1 and 2, Closes: on the last one
- New patch 3 from Quanye for the Netlink path, per the discussion
  with Matt [2]

Validated with the virtme CI (mptcp selftests), and with a stress
repro comparing the base kernel and the patched one: on the base
kernel the server accepts 256 MP_JOINs and the counter wraps to 0,
with the patches admission stops at 255 and the leftover imbalance
only shows up as a rate-limited warning.

[1] https://lore.kernel.org/20260831093206.689827-1-cui.tao@linux.dev/
[2] https://lore.kernel.org/all/c8c4f667-5d25-4d94-8627-387d4091c8c4@kernel.org/

Quanye Yang (1):
  mptcp: pm: userspace: cap extra_subflows on Netlink subflow creation

Tao Cui (2):
  mptcp: pm: bound extra_subflows admission on userspace PM
  mptcp: pm: skip extra_subflows accounting on disconnected msk

 net/mptcp/pm.c           | 17 ++++++++++++++---
 net/mptcp/pm_userspace.c |  7 +++++++
 net/mptcp/protocol.h     | 14 ++++++++++++--
 3 files changed, 33 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH mptcp-next v3 1/3] mptcp: pm: bound extra_subflows admission on userspace PM

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

mptcp_pm_allow_new_subflow() increments the u8 extra_subflows counter
for every accepted MP_JOIN on sockets using the userspace PM, without
any limit. A peer establishing more than 255 live subflows wraps the
counter back to 0, which then makes the underflow guards warn on the
next subflow close, and permanently corrupts mptcpi_subflows_total
reported to userspace.

Refuse new MP_JOINs once the counter has reached U8_MAX, so that it
cannot wrap anymore.

Fixes: e99c1ca89071 ("mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow")
Link: https://github.com/multipath-tcp/mptcp_net-next/issues/629
Co-developed-by: Quanye Yang <quanyeyang@proton.me>
Signed-off-by: Quanye Yang <quanyeyang@proton.me>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 net/mptcp/pm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 8b68868255c5..07cdcdb54b15 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -563,9 +563,11 @@ bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
 	if (mptcp_pm_is_userspace(msk)) {
 		if (mptcp_userspace_pm_active(msk)) {
 			spin_lock_bh(&pm->lock);
-			pm->extra_subflows++;
+			ret = pm->extra_subflows < U8_MAX;
+			if (ret)
+				pm->extra_subflows++;
 			spin_unlock_bh(&pm->lock);
-			return true;
+			return ret;
 		}
 		return false;
 	}
-- 
2.43.0

[PATCH mptcp-next v3 2/3] mptcp: pm: skip extra_subflows accounting on disconnected msk

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

The WARN_ON_ONCE() guards added to the extra_subflows decrement sites
turn out to be reachable:

mptcp_pm_data_reset() zeroes the counter with only the msk socket lock
held, while an MP_JOIN subflow can still sit in msk->join_list, its
reference already accounted by mptcp_pm_allow_new_subflow() under
pm->lock. If the socket gets disconnected(AF_UNSPEC) in that window,
mptcp_pm_data_reset() zeroes the counter, and the join list is flushed
later at release_sock() time: the leftover subflow then reaches
mptcp_pm_subflow_check_next() (or __mptcp_pm_close_subflow() for
kernel PM sockets) with the counter already at 0, firing the warning.
On panic_on_warn kernels this is a remotely triggerable panic, which
is worse than the silent wrap the guards replaced.

Skip the PM accounting when the msk is already in TCP_CLOSE: in the
scenario above the state is set before the counters are cleared, and
once the msk is closed the accounting is not relevant anymore. Keep a
clamp and a rate-limited pr_warn() on the decrement sites instead,
to leave a trace of any imbalance we would still not know about.

Fixes: e99c1ca89071 ("mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow")
Link: https://github.com/multipath-tcp/mptcp_net-next/issues/629
Suggested-by: Matthieu Baerts <matttbe@kernel.org>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 net/mptcp/pm.c       | 11 ++++++++++-
 net/mptcp/protocol.h | 14 ++++++++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 07cdcdb54b15..cccf5319cce0 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -671,9 +671,18 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
 	update_subflows = subflow->request_join || subflow->mp_join;
 	if (mptcp_pm_is_userspace(msk)) {
 		if (update_subflows) {
+			/* The PM counters have already been cleared if the
+			 * msk got disconnected while this subflow was still
+			 * queued in the join list
+			 */
+			if (inet_sk_state_load(sk) == TCP_CLOSE)
+				return;
 			spin_lock_bh(&pm->lock);
-			if (!WARN_ON_ONCE(pm->extra_subflows == 0))
+			if (likely(pm->extra_subflows))
 				pm->extra_subflows--;
+			else
+				pr_warn_ratelimited("extra_subflows underflow, msk=%p\n",
+						    msk);
 			spin_unlock_bh(&pm->lock);
 		}
 		return;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 87ccb84e9927..07821551499c 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1252,8 +1252,18 @@ u8 mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk);
 /* called under PM lock */
 static inline void __mptcp_pm_close_subflow(struct mptcp_sock *msk)
 {
-	if (!WARN_ON_ONCE(msk->pm.extra_subflows == 0) &&
-	    --msk->pm.extra_subflows < mptcp_pm_get_limit_extra_subflows(msk))
+	/* The PM counters have already been cleared if the msk got
+	 * disconnected while this subflow was still queued in the
+	 * join list
+	 */
+	if (inet_sk_state_load((struct sock *)msk) == TCP_CLOSE)
+		return;
+
+	if (unlikely(msk->pm.extra_subflows == 0)) {
+		pr_warn_ratelimited("extra_subflows underflow, msk=%p\n", msk);
+		return;
+	}
+	if (--msk->pm.extra_subflows < mptcp_pm_get_limit_extra_subflows(msk))
 		WRITE_ONCE(msk->pm.accept_subflow, true);
 }
 
-- 
2.43.0

[PATCH mptcp-next v3 3/3] mptcp: pm: userspace: cap extra_subflows on Netlink subflow creation

[Tao Cui]

From: Quanye Yang <quanyeyang@proton.me>

mptcp_pm_nl_subflow_create_doit() also increments the u8 extra_subflows
counter without any limit: a client with CAP_NET_ADMIN can create more
than 255 extra subflows via Netlink and wrap the counter back to 0,
with the same effects as the MP_JOIN path now bounded separately:
the underflow guards warn on the next subflow close, and
mptcpi_subflows_total reported to userspace is corrupted.

Refuse the subflow creation with -ENOSPC once the counter has reached
U8_MAX, and clean up the local address entry in that case as well.

The same error path is followed after
mptcp_userspace_pm_append_new_local_addr, which should probably not
delete the local address unconditionally, but this is a pre-existing
issue that will be addressed separately.

Fixes: 77e4b94a3de6 ("mptcp: update userspace pm infos")
Link: https://lore.kernel.org/all/20260902-mptcp-pm-extra-subflows-v1-1-68540a866e5a@proton.me/
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/629
Signed-off-by: Quanye Yang <quanyeyang@proton.me>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 net/mptcp/pm_userspace.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index b94fbb483bf9..e7f7d023c246 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -417,6 +417,12 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	local.ifindex = entry.ifindex;
 
 	spin_lock_bh(&msk->pm.lock);
+	if (msk->pm.extra_subflows == U8_MAX) {
+		spin_unlock_bh(&msk->pm.lock);
+		GENL_SET_ERR_MSG(info, "too many extra subflows");
+		err = -ENOSPC;
+		goto delete_addr;
+	}
 	msk->pm.extra_subflows++;
 	spin_unlock_bh(&msk->pm.lock);
 
@@ -427,6 +433,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	if (err) {
 		GENL_SET_ERR_MSG_FMT(info, "connect error: %d", err);
 
+delete_addr:
 		spin_lock_bh(&msk->pm.lock);
 		mptcp_userspace_pm_delete_local_addr(msk, &entry);
 		spin_unlock_bh(&msk->pm.lock);
-- 
2.43.0

Re: [PATCH mptcp-next v3 2/3] mptcp: pm: skip extra_subflows accounting on disconnected msk

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Data race in mptcp_pm_data_reset() when clearing the pm->reset struct group locklessly, causing torn reads/writes with concurrent PM operations.
--

commit 0c791790f3c719050b1d2004c347405ffb76512f
Author: Tao Cui <cuitao@kylinos.cn>

mptcp: pm: skip extra_subflows accounting on disconnected msk

This commit addresses a remotely triggerable panic caused by a WARN_ON_ONCE
guard on extra_subflows underflow. It skips the PM accounting when the socket
is in TCP_CLOSE, as the counters may have already been cleared by
mptcp_pm_data_reset().

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

> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 9881092c16e78..520f0c638e6f7 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -671,9 +671,18 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
>  	update_subflows = subflow->request_join || subflow->mp_join;
>  	if (mptcp_pm_is_userspace(msk)) {
>  		if (update_subflows) {
> +			/* The PM counters have already been cleared if the
> +			 * msk got disconnected while this subflow was still
> +			 * queued in the join list
> +			 */

[Severity: High]
This isn't a bug introduced by this patch, but is there a data race when
clearing the pm->reset struct group locklessly in mptcp_pm_data_reset()?

Looking at net/mptcp/protocol.c:mptcp_disconnect(), it calls
mptcp_pm_data_reset() on an active, network-exposed socket without acquiring
pm->lock:

net/mptcp/protocol.c:mptcp_disconnect() {
    ...
    mptcp_pm_data_reset(msk);
    ...
}

Inside net/mptcp/pm.c:mptcp_pm_data_reset(), the code does a lockless memset
of the struct group:

net/mptcp/pm.c:mptcp_pm_data_reset() {
    ...
    memset(&pm->reset, 0, sizeof(pm->reset));
    ...
}

Could this cause torn reads and writes with concurrent PM operations?

For example, mptcp_pm_allow_new_subflow() concurrently modifies the memory
while holding pm->lock, but without holding the socket lock:

net/mptcp/pm.c:mptcp_pm_allow_new_subflow() {
    ...
    pm->extra_subflows++;
    ...
}

If a client socket initiates a local disconnect via connect(AF_UNSPEC), and
concurrently an in-flight SYN-ACK or remote MP_JOIN request is processed in
softirq context via mptcp_finish_join(), could they interleave?

It seems this might lead to data corruption of the pm->reset struct group
fields (including extra_subflows, accept_subflow, and addr_signal), which
could cause PM misbehavior such as permanent rejection of new subflows on a
reconnected socket.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904051150.1196427-1-cui.tao@linux.dev?part=2

Re: [PATCH mptcp-next v3 0/3] mptcp: pm: fix reachable extra_subflows guards

[MPTCP CI]

Hi Quanye,

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/33840713883

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


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)