All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow
@ 2026-05-22  8:50 Tao Cui
  2026-05-22 10:31 ` MPTCP CI
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Tao Cui @ 2026-05-22  8:50 UTC (permalink / raw)
  To: matttbe, geliang; +Cc: mptcp, 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


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

* Re: [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow
  2026-05-22  8:50 [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow Tao Cui
@ 2026-05-22 10:31 ` MPTCP CI
  2026-05-27 11:40 ` Matthieu Baerts
  2026-08-26 13:52 ` Matthieu Baerts
  2 siblings, 0 replies; 7+ messages in thread
From: MPTCP CI @ 2026-05-22 10:31 UTC (permalink / raw)
  To: Tao Cui; +Cc: mptcp

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)

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

* Re: [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow
  2026-05-22  8:50 [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow Tao Cui
  2026-05-22 10:31 ` MPTCP CI
@ 2026-05-27 11:40 ` Matthieu Baerts
  2026-08-26 13:52 ` Matthieu Baerts
  2 siblings, 0 replies; 7+ messages in thread
From: Matthieu Baerts @ 2026-05-27 11:40 UTC (permalink / raw)
  To: Tao Cui, geliang; +Cc: mptcp

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.


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

* [PATCH net-next 01/11] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow
  2026-08-12 14:55 [PATCH net-next 00/11] mptcp: misc. features for v7.3 Matthieu Baerts (NGI0)
@ 2026-08-12 14:55 ` Matthieu Baerts (NGI0)
  0 siblings, 0 replies; 7+ messages in thread
From: Matthieu Baerts (NGI0) @ 2026-08-12 14:55 UTC (permalink / raw)
  To: Mat Martineau, Geliang Tang, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman
  Cc: netdev, mptcp, linux-kernel, Matthieu Baerts (NGI0), Tao Cui

From: Tao Cui <cuitao@kylinos.cn>

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>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 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 d1f73c3e39fa..8b68868255c5 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -670,7 +670,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 b3af3462bdd1..20627e12c113 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1254,7 +1254,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.53.0


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

* Re: [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow
  2026-05-22  8:50 [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow Tao Cui
  2026-05-22 10:31 ` MPTCP CI
  2026-05-27 11:40 ` Matthieu Baerts
@ 2026-08-26 13:52 ` Matthieu Baerts
  2026-08-28  8:33   ` [PATCH net-next 01/11] " Tao Cui
  2 siblings, 1 reply; 7+ messages in thread
From: Matthieu Baerts @ 2026-08-26 13:52 UTC (permalink / raw)
  To: Tao Cui, geliang; +Cc: mptcp

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.


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

* Re: [PATCH net-next 01/11] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow
  2026-08-26 13:52 ` Matthieu Baerts
@ 2026-08-28  8:33   ` Tao Cui
  2026-08-28  9:14     ` Matthieu Baerts
  0 siblings, 1 reply; 7+ messages in thread
From: Tao Cui @ 2026-08-28  8:33 UTC (permalink / raw)
  To: matttbe; +Cc: cuitao, geliang, mptcp, cui.tao

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

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

* Re: [PATCH net-next 01/11] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow
  2026-08-28  8:33   ` [PATCH net-next 01/11] " Tao Cui
@ 2026-08-28  9:14     ` Matthieu Baerts
  0 siblings, 0 replies; 7+ messages in thread
From: Matthieu Baerts @ 2026-08-28  9:14 UTC (permalink / raw)
  To: Tao Cui; +Cc: cuitao, geliang, mptcp

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.


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

end of thread, other threads:[~2026-08-28  9:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-22  8:50 [PATCH mptcp-next] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow Tao Cui
2026-05-22 10:31 ` MPTCP CI
2026-05-27 11:40 ` Matthieu Baerts
2026-08-26 13:52 ` Matthieu Baerts
2026-08-28  8:33   ` [PATCH net-next 01/11] " Tao Cui
2026-08-28  9:14     ` Matthieu Baerts
  -- strict thread matches above, loose matches on Subject: below --
2026-08-12 14:55 [PATCH net-next 00/11] mptcp: misc. features for v7.3 Matthieu Baerts (NGI0)
2026-08-12 14:55 ` [PATCH net-next 01/11] mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow Matthieu Baerts (NGI0)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.