[PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow

[PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow

[Tao Cui]

extra_subflows is a u8 counter that can underflow if a decrement races
with or precedes an increment. While the recently fixed userspace PM
subflow creation path eliminated the primary cause, add defensive
WARN_ON_ONCE guards at both decrement sites to catch any remaining edge
cases rather than silently wrapping to 255.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 net/mptcp/pm.c       | 3 ++-
 net/mptcp/protocol.h | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 3c152bf66cd5..aa987366fb24 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -655,7 +655,8 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
 	if (mptcp_pm_is_userspace(msk)) {
 		if (update_subflows) {
 			spin_lock_bh(&pm->lock);
-			pm->extra_subflows--;
+			if (!WARN_ON_ONCE(pm->extra_subflows == 0))
+				pm->extra_subflows--;
 			spin_unlock_bh(&pm->lock);
 		}
 		return;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index e4f5aba24da7..2cf6e3f66a95 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1276,7 +1276,8 @@ 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 (--msk->pm.extra_subflows < mptcp_pm_get_limit_extra_subflows(msk))
+	if (!WARN_ON_ONCE(msk->pm.extra_subflows == 0) &&
+	    --msk->pm.extra_subflows < mptcp_pm_get_limit_extra_subflows(msk))
 		WRITE_ONCE(msk->pm.accept_subflow, true);
 }
 
-- 
2.43.0

Re: [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow

[MPTCP CI]

Hi Tao,

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

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


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)

Re: [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow

[Matthieu Baerts]

Hi Tao,

On 22/05/2026 18:50, Tao Cui wrote:
> extra_subflows is a u8 counter that can underflow if a decrement races
> with or precedes an increment. While the recently fixed userspace PM
> subflow creation path eliminated the primary cause, add defensive
> WARN_ON_ONCE guards at both decrement sites to catch any remaining edge
> cases rather than silently wrapping to 255.
Thank you for the patch:

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

New patches for t/upstream:
- abefe4ecdb46: mptcp: pm: add WARN_ON_ONCE guards on extra_subflows
underflow
- Results: c40ac68d9bc2..cde220b5fe28 (export)

Tests are now in progress:

- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/02fc64d2cebb020984c74fb4950be7a46909b4d9/checks

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

Re: [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow

[Matthieu Baerts]

Hi Tao,

On 22/05/2026 10:50, Tao Cui wrote:
> extra_subflows is a u8 counter that can underflow if a decrement races
> with or precedes an increment. While the recently fixed userspace PM
> subflow creation path eliminated the primary cause, add defensive
> WARN_ON_ONCE guards at both decrement sites to catch any remaining edge
> cases rather than silently wrapping to 255.

FYI, Clashiko found some existing issues linked to this patch:

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

I don't know if it impacts your case, but just to avoid having multiple
people looking at it, do you plan to address Clashiko's comments?

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

Re: [PATCH net-next 01/11] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Hi Matt,

> On 22/05/2026 10:50, Tao Cui wrote:
>> extra_subflows is a u8 counter that can underflow if a decrement races
>> with or precedes an increment. While the recently fixed userspace PM
>> subflow creation path eliminated the primary cause, add defensive
>> WARN_ON_ONCE guards at both decrement sites to catch any remaining edge
>> cases rather than silently wrapping to 255.
> 
> FYI, Clashiko found some existing issues linked to this patch:
> 
> https://github.com/multipath-tcp/mptcp_net-next/issues/629
> 
> I don't know if it impacts your case, but just to avoid having multiple
> people looking at it, do you plan to address Clashiko's comments?

Thanks for the pointer, I had missed that issue.

I had another look and yes, I'll take care of it. Both findings look
real to me:

The disconnect() race is the nasty one. The MP_JOIN softirq bumps the
counter under pm->lock and defers the subflow to the join list, then
mptcp_pm_data_reset() zeroes it with only the socket lock held, and
the subflow gets closed later at release_sock() time when the join
list is flushed. So we hit the new warn with the counter already at 0.
With panic_on_warn that's a remotely triggerable panic, which is
arguably worse than the silent wrap we had before.

The unbounded increment on the userspace PM side is pre-existing, but
now the wrap also lands on the warn instead of just corrupting
mptcpi_subflows_total.

I don't think we want to keep WARN_ON_ONCE() on paths we know are
reachable. My plan for a fix series:

- refuse new MP_JOINs on the userspace PM once extra_subflows is at
  U8_MAX, so the counter can't wrap anymore
- sort out the accounting across disconnect (reset vs join list
  drain) and downgrade the warn to a clamp, maybe with a
  pr_warn_ratelimited() to keep some trace of it

I'll follow up in the issue once I have patches. No strong opinion
between clamping and refusing admission at the limit, happy to go
with whatever you prefer.

Thanks,
Tao

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

Re: [PATCH net-next 01/11] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow

[Matthieu Baerts]

Hi Tao,

On 28/08/2026 10:33, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> Hi Matt,
> 
>> On 22/05/2026 10:50, Tao Cui wrote:
>>> extra_subflows is a u8 counter that can underflow if a decrement races
>>> with or precedes an increment. While the recently fixed userspace PM
>>> subflow creation path eliminated the primary cause, add defensive
>>> WARN_ON_ONCE guards at both decrement sites to catch any remaining edge
>>> cases rather than silently wrapping to 255.
>>
>> FYI, Clashiko found some existing issues linked to this patch:
>>
>> https://github.com/multipath-tcp/mptcp_net-next/issues/629
>>
>> I don't know if it impacts your case, but just to avoid having multiple
>> people looking at it, do you plan to address Clashiko's comments?
> 
> Thanks for the pointer, I had missed that issue.
> 
> I had another look and yes, I'll take care of it. Both findings look
> real to me:
> 
> The disconnect() race is the nasty one. The MP_JOIN softirq bumps the
> counter under pm->lock and defers the subflow to the join list, then
> mptcp_pm_data_reset() zeroes it with only the socket lock held, and
> the subflow gets closed later at release_sock() time when the join
> list is flushed. So we hit the new warn with the counter already at 0.
> With panic_on_warn that's a remotely triggerable panic, which is
> arguably worse than the silent wrap we had before.
> 
> The unbounded increment on the userspace PM side is pre-existing, but
> now the wrap also lands on the warn instead of just corrupting
> mptcpi_subflows_total.
> 
> I don't think we want to keep WARN_ON_ONCE() on paths we know are
> reachable. My plan for a fix series:
> 
> - refuse new MP_JOINs on the userspace PM once extra_subflows is at
>   U8_MAX, so the counter can't wrap anymore
> - sort out the accounting across disconnect (reset vs join list
>   drain) and downgrade the warn to a clamp, maybe with a
>   pr_warn_ratelimited() to keep some trace of it
> 
> I'll follow up in the issue once I have patches. No strong opinion
> between clamping and refusing admission at the limit, happy to go
> with whatever you prefer.
Thank you for having checked and looking at fixes!

Note that for the disconnect part, maybe other variables could be
checked before looking at decrementing the PM counters? e.g. the msk
state? Would that work?

Note that I think we would prefer a pr_warn_ratelimited over a complex
fix involving more locks.

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