Re: [PATCH mptcp-next v2 1/5] mptcp: sched: change scheduler sysctl atomically

Hi Gang,

On 18/08/2026 14:56, [email protected] wrote:
> August 18, 2026 at 5:59 PM, [email protected] mailto:[email protected]  wrote:
> 
> 
>>
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>> - [High] Use-After-Free and module reference leak due to missing reference counting when setting the default MPTCP scheduler
>> - [Medium] Redundant scheduler list traversal during socket initialization defeats pointer caching
>> --
>>
>> --- Patch [1]: [PATCH mptcp-next v2 1/5] mptcp: sched: change scheduler sysctl atomically ---
>> commit d5b1780c033b9909a1b30fbd814812af5c25ed2f
>> Author: Gang Yan <[email protected]>
>>
>> mptcp: sched: change scheduler sysctl atomically
>>
>> This commit changes the per-netns scheduler storage from an inline string
>> to an RCU-protected pointer to an mptcp_sched_ops struct. It updates the
>> sysctl read/write paths and socket initialization to use this pointer,
>> aiming to prevent concurrent readers from observing half-written names.
>>
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/626
>>
>>>
>>> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
>>>  index 63c5747f0f63c..479b31eb30079 100644
>>>  --- a/net/mptcp/ctrl.c
>>>  +++ b/net/mptcp/ctrl.c
>>>
>> [ ... ]
>>
>>>
>>> @@ -126,7 +133,7 @@ static int mptcp_set_scheduler(char *scheduler, const char *name)
>>>  rcu_read_lock();
>>>  sched = mptcp_sched_find(name);
>>>  if (sched)
>>>  - strscpy(scheduler, name, MPTCP_SCHED_NAME_MAX);
>>>  + xchg(&pernet->scheduler, sched);
>>>
>> [Severity: High]
>> When replacing the scheduler pointer here with xchg(), does this leak the
>> module reference for the previous scheduler returned by xchg()?
>>
>> Also, do we need to acquire a module reference for the newly assigned sched
>> via bpf_try_module_get() before storing it? Without a reference, could the
>> module be unloaded, leaving pernet->scheduler pointing to freed memory that
>> would trigger a use-after-free when dereferenced by a concurrent reader in
>> proc_scheduler()?
> 
> Hi Matt,

(you can address such email to the list, I'm not the only one here ;) )

> As far as I know, for linux-master branch, the sched ops cannot be torndown, and
> the only static default scheduler is used, so no reference counting is needed.

Thank you for replying to these emails from Sashiko, that's the right
way to do!

> It is safe for now, the pm is as the same. Right?

Oh, sorry, I just realised the BPF module part for the scheduler is
already in Linux mainstream [1], but not the PM one [2]:

[1] https://elixir.bootlin.com/linux/v7.2/source/net/mptcp/sched.c#L130-L160
[2] https://elixir.bootlin.com/linux/v7.2/source/net/mptcp/pm.c#L1225-L1254

Sorry for the confusion, but then I guess the split is only needed for
the PM side. In fact patch 1/3 from the v1 can apply on net-next
directly. So in terms of split, the sched part from v1 was OK (no
split), but for the PM, we need the split from v2.

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