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

[Matthieu Baerts <matttbe@kernel.org>]

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.