[PATCH mptcp-next v2 0/5] mptcp: avoid data-races around the sysctls

[PATCH mptcp-next v2 0/5] mptcp: avoid data-races around the sysctls

[Gang Yan]

From: Gang Yan <yangang@kylinos.cn>

Changelog:
v2:
  - Split the code around bpf to patch 4 and patch 5.
  - Put WRITE_ONCE(pernet->pm_type, pm_type) into patch 3 and add
    tag of Tao Cui.
v1:
  Link: https://patchwork.kernel.org/project/mptcp/cover/20260817012452.7519-1-gang.yan@linux.dev/


Gang Yan (4):
  mptcp: sched: change scheduler sysctl atomically
  mptcp: pm: change path_manager sysctl atomically
  Squash to "mptcp: pm: init and release mptcp_pm_ops"
  Squash to "bpf: Add mptcp packet scheduler struct_ops"

Matthieu Baerts (NGI0) (1):
  mptcp: use READ_ONCE() over sysctls

 net/mptcp/ctrl.c     | 116 ++++++++++++++++++++++++++++++-------------
 net/mptcp/pm.c       |   3 +-
 net/mptcp/protocol.c |   5 +-
 net/mptcp/protocol.h |   6 +--
 net/mptcp/sched.c    |   2 +-
 5 files changed, 91 insertions(+), 41 deletions(-)

-- 
2.43.0

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

[Gang Yan]

From: Gang Yan <yangang@kylinos.cn>

The per-netns scheduler name is stored as an inline char[] buffer and
updated via strscpy() from the sysctl handler. A concurrent reader (e.g.
mptcp_init_sock() resolving the default scheduler) can observe a
half-written name, which is also flagged by KCSAN. READ_ONCE() does not
help here as it cannot read a multi-byte string atomically.

Following the tcp_congestion_control() model, store a pointer to the
immutable struct mptcp_sched_ops instead of the name string:
  - mptcp_set_scheduler() now looks the ops up and atomically swaps the
    pernet pointer with xchg();
  - mptcp_get_scheduler() copies the ops name out under rcu_read_lock();
  - the default ops is assigned in mptcp_pernet_set_defaults().

A pointer store is a single atomic word, so readers always observe a
consistent value.

Assisted-by: Claude:GLM5.2
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/626
Co-developed-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 net/mptcp/ctrl.c     | 31 ++++++++++++++++++++++---------
 net/mptcp/protocol.c |  5 +++--
 net/mptcp/protocol.h |  3 ++-
 net/mptcp/sched.c    |  2 +-
 4 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 63c5747f0f63..479b31eb3007 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -39,7 +39,7 @@ struct mptcp_pernet {
 	u8 allow_join_initial_addr_port;
 	u8 pm_type;
 	u8 add_addr_v6_port_drop_ts;
-	char scheduler[MPTCP_SCHED_NAME_MAX];
+	struct mptcp_sched_ops __rcu *scheduler;
 	char path_manager[MPTCP_PM_NAME_MAX];
 };
 
@@ -90,9 +90,14 @@ const char *mptcp_get_path_manager(const struct net *net)
 	return mptcp_get_pernet(net)->path_manager;
 }
 
-const char *mptcp_get_scheduler(const struct net *net)
+void mptcp_get_scheduler(const struct net *net, char *name)
 {
-	return mptcp_get_pernet(net)->scheduler;
+	struct mptcp_sched_ops *sched;
+
+	rcu_read_lock();
+	sched = rcu_dereference(mptcp_get_pernet(net)->scheduler);
+	strscpy(name, sched ? sched->name : "default", MPTCP_SCHED_NAME_MAX);
+	rcu_read_unlock();
 }
 
 unsigned int mptcp_add_addr_v6_port_drop_ts(const struct net *net)
@@ -112,13 +117,15 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
 	pernet->allow_join_initial_addr_port = 1;
 	pernet->stale_loss_cnt = 4;
 	pernet->pm_type = MPTCP_PM_TYPE_KERNEL;
-	strscpy(pernet->scheduler, "default", sizeof(pernet->scheduler));
+
+	RCU_INIT_POINTER(pernet->scheduler, &mptcp_sched_default);
+
 	strscpy(pernet->path_manager, "kernel", sizeof(pernet->path_manager));
 	pernet->add_addr_v6_port_drop_ts = 1;
 }
 
 #ifdef CONFIG_SYSCTL
-static int mptcp_set_scheduler(char *scheduler, const char *name)
+static int mptcp_set_scheduler(struct mptcp_pernet *pernet, const char *name)
 {
 	struct mptcp_sched_ops *sched;
 	int ret = 0;
@@ -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);
 	else
 		ret = -ENOENT;
 	rcu_read_unlock();
@@ -137,7 +144,10 @@ static int mptcp_set_scheduler(char *scheduler, const char *name)
 static int proc_scheduler(const struct ctl_table *ctl, int write,
 			  void *buffer, size_t *lenp, loff_t *ppos)
 {
-	char (*scheduler)[MPTCP_SCHED_NAME_MAX] = ctl->data;
+	struct mptcp_pernet *pernet = container_of(ctl->data,
+						   struct mptcp_pernet,
+						   scheduler);
+	struct mptcp_sched_ops *sched;
 	char val[MPTCP_SCHED_NAME_MAX];
 	struct ctl_table tbl = {
 		.data = val,
@@ -145,11 +155,14 @@ static int proc_scheduler(const struct ctl_table *ctl, int write,
 	};
 	int ret;
 
-	strscpy(val, *scheduler, MPTCP_SCHED_NAME_MAX);
+	rcu_read_lock();
+	sched = rcu_dereference(pernet->scheduler);
+	strscpy(val, sched ? sched->name : "default", MPTCP_SCHED_NAME_MAX);
+	rcu_read_unlock();
 
 	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
 	if (write && ret == 0)
-		ret = mptcp_set_scheduler(*scheduler, val);
+		ret = mptcp_set_scheduler(pernet, val);
 
 	return ret;
 }
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f879b1061f2d..9d84dd803802 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3260,6 +3260,7 @@ static void mptcp_ca_reset(struct sock *sk)
 static int mptcp_init_sock(struct sock *sk)
 {
 	struct net *net = sock_net(sk);
+	char sched_name[MPTCP_SCHED_NAME_MAX];
 	int ret;
 
 	__mptcp_init_sock(sk);
@@ -3271,8 +3272,8 @@ static int mptcp_init_sock(struct sock *sk)
 		return -ENOMEM;
 
 	rcu_read_lock();
-	ret = mptcp_init_sched(mptcp_sk(sk),
-			       mptcp_sched_find(mptcp_get_scheduler(net)));
+	mptcp_get_scheduler(net, sched_name);
+	ret = mptcp_init_sched(mptcp_sk(sk), mptcp_sched_find(sched_name));
 	rcu_read_unlock();
 	if (ret)
 		return ret;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 7e168e450fb0..af79b3450ab7 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -803,7 +803,7 @@ unsigned int mptcp_stale_loss_cnt(const struct net *net);
 unsigned int mptcp_close_timeout(const struct sock *sk);
 int mptcp_get_pm_type(const struct net *net);
 const char *mptcp_get_path_manager(const struct net *net);
-const char *mptcp_get_scheduler(const struct net *net);
+void mptcp_get_scheduler(const struct net *net, char *name);
 unsigned int mptcp_add_addr_v6_port_drop_ts(const struct net *net);
 
 void mptcp_active_disable(struct sock *sk);
@@ -1155,6 +1155,7 @@ int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_
 
 /* the default path manager, used in mptcp_pm_unregister */
 extern struct mptcp_pm_ops mptcp_pm_kernel;
+extern struct mptcp_sched_ops mptcp_sched_default;
 
 struct mptcp_pm_ops *mptcp_pm_find(const char *name);
 int mptcp_pm_register(struct mptcp_pm_ops *pm_ops);
diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
index 1e59072d478c..0d13ee46ffdf 100644
--- a/net/mptcp/sched.c
+++ b/net/mptcp/sched.c
@@ -40,7 +40,7 @@ static int mptcp_sched_default_get_retrans(struct mptcp_sock *msk)
 	return 0;
 }
 
-static struct mptcp_sched_ops mptcp_sched_default = {
+struct mptcp_sched_ops mptcp_sched_default = {
 	.get_send	= mptcp_sched_default_get_send,
 	.get_retrans	= mptcp_sched_default_get_retrans,
 	.name		= "default",
-- 
2.43.0

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

[sashiko-bot]

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 <yangang@kylinos.cn>

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()?

[ ... ]
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index c2762d74f29d7..82df0400d0f94 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
[ ... ]
> @@ -3271,8 +3272,8 @@ static int mptcp_init_sock(struct sock *sk)
>  		return -ENOMEM;
>  
>  	rcu_read_lock();
> -	ret = mptcp_init_sched(mptcp_sk(sk),
> -			       mptcp_sched_find(mptcp_get_scheduler(net)));
> +	mptcp_get_scheduler(net, sched_name);
> +	ret = mptcp_init_sched(mptcp_sk(sk), mptcp_sched_find(sched_name));

[Severity: Medium]
Since pernet->scheduler now stores the mptcp_sched_ops pointer directly,
could we retrieve and use that pointer here directly? Extracting the name
via mptcp_get_scheduler() just to perform a redundant list traversal with
mptcp_sched_find() seems to defeat the purpose of caching the operations
pointer.

>  	rcu_read_unlock();
>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818094825.48446-1-gang.yan@linux.dev?part=1

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

[gang.yan]

August 18, 2026 at 5:59 PM, sashiko-bot@kernel.org mailto:sashiko-bot@kernel.org  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 <yangang@kylinos.cn>
> 
> 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,

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.

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

> 
> [ ... ]
> 
> > 
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> >  index c2762d74f29d7..82df0400d0f94 100644
> >  --- a/net/mptcp/protocol.c
> >  +++ b/net/mptcp/protocol.c
> > 
> [ ... ]
> 
> > 
> > @@ -3271,8 +3272,8 @@ static int mptcp_init_sock(struct sock *sk)
> >  return -ENOMEM;
> >  
> >  rcu_read_lock();
> >  - ret = mptcp_init_sched(mptcp_sk(sk),
> >  - mptcp_sched_find(mptcp_get_scheduler(net)));
> >  + mptcp_get_scheduler(net, sched_name);
> >  + ret = mptcp_init_sched(mptcp_sk(sk), mptcp_sched_find(sched_name));
> > 
> [Severity: Medium]
> Since pernet->scheduler now stores the mptcp_sched_ops pointer directly,
> could we retrieve and use that pointer here directly? Extracting the name
> via mptcp_get_scheduler() just to perform a redundant list traversal with
> mptcp_sched_find() seems to defeat the purpose of caching the operations
> pointer.
>

That's a good point, will done in v3.

Thanks
Gang
 
> > 
> > rcu_read_unlock();
> >  if (ret)
> >  return ret;
> > 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260818094825.48446-1-gang.yan@linux.dev?part=1
>

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

[Matthieu Baerts]

Hi Gang,

On 18/08/2026 14:56, gang.yan@linux.dev wrote:
> August 18, 2026 at 5:59 PM, sashiko-bot@kernel.org mailto:sashiko-bot@kernel.org  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 <yangang@kylinos.cn>
>>
>> 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.

[PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically

[Gang Yan]

From: Gang Yan <yangang@kylinos.cn>

The per-netns path manager name is stored as an inline char[] buffer and
updated via strscpy() from the sysctl handler; a concurrent reader can
observe a half-written name (and KCSAN flags the race), which READ_ONCE()
cannot fix for a multi-byte string.

Following the tcp_congestion_control() model (and the scheduler change in
the previous patch), store a pointer to the immutable struct mptcp_pm_ops
instead of the name string:
  - mptcp_set_path_manager() looks the ops up and atomically swaps the
    pernet pointer with xchg();
  - mptcp_get_path_manager() copies the ops name out under rcu_read_lock();
  - the default ops is assigned in mptcp_pernet_set_defaults().

Assisted-by: Claude:GLM5.2
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/626
Co-developed-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 net/mptcp/ctrl.c     | 28 ++++++++++++++++++----------
 net/mptcp/pm.c       |  3 ++-
 net/mptcp/protocol.h |  3 +--
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 479b31eb3007..c0481b09c1a1 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -40,7 +40,7 @@ struct mptcp_pernet {
 	u8 pm_type;
 	u8 add_addr_v6_port_drop_ts;
 	struct mptcp_sched_ops __rcu *scheduler;
-	char path_manager[MPTCP_PM_NAME_MAX];
+	struct mptcp_pm_ops __rcu *path_manager;
 };
 
 static struct mptcp_pernet *mptcp_get_pernet(const struct net *net)
@@ -85,9 +85,14 @@ int mptcp_get_pm_type(const struct net *net)
 	return mptcp_get_pernet(net)->pm_type;
 }
 
-const char *mptcp_get_path_manager(const struct net *net)
+void mptcp_get_path_manager(const struct net *net, char *name)
 {
-	return mptcp_get_pernet(net)->path_manager;
+	struct mptcp_pm_ops *pm_ops;
+
+	rcu_read_lock();
+	pm_ops = rcu_dereference(mptcp_get_pernet(net)->path_manager);
+	strscpy(name, pm_ops ? pm_ops->name : "kernel", MPTCP_PM_NAME_MAX);
+	rcu_read_unlock();
 }
 
 void mptcp_get_scheduler(const struct net *net, char *name)
@@ -119,8 +124,8 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
 	pernet->pm_type = MPTCP_PM_TYPE_KERNEL;
 
 	RCU_INIT_POINTER(pernet->scheduler, &mptcp_sched_default);
+	RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);
 
-	strscpy(pernet->path_manager, "kernel", sizeof(pernet->path_manager));
 	pernet->add_addr_v6_port_drop_ts = 1;
 }
 
@@ -201,7 +206,7 @@ static int proc_blackhole_detect_timeout(const struct ctl_table *table,
 	return ret;
 }
 
-static int mptcp_set_path_manager(char *path_manager, const char *name)
+static int mptcp_set_path_manager(struct mptcp_pernet *pernet, const char *name)
 {
 	struct mptcp_pm_ops *pm_ops;
 	int ret = 0;
@@ -209,7 +214,7 @@ static int mptcp_set_path_manager(char *path_manager, const char *name)
 	rcu_read_lock();
 	pm_ops = mptcp_pm_find(name);
 	if (pm_ops)
-		strscpy(path_manager, name, MPTCP_PM_NAME_MAX);
+		xchg(&pernet->path_manager, pm_ops);
 	else
 		ret = -ENOENT;
 	rcu_read_unlock();
@@ -223,7 +228,7 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
 	struct mptcp_pernet *pernet = container_of(ctl->data,
 						   struct mptcp_pernet,
 						   path_manager);
-	char (*path_manager)[MPTCP_PM_NAME_MAX] = ctl->data;
+	struct mptcp_pm_ops *pm_ops;
 	char pm_name[MPTCP_PM_NAME_MAX];
 	const struct ctl_table tbl = {
 		.data = pm_name,
@@ -231,11 +236,14 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
 	};
 	int ret;
 
-	strscpy(pm_name, *path_manager, MPTCP_PM_NAME_MAX);
+	rcu_read_lock();
+	pm_ops = rcu_dereference(pernet->path_manager);
+	strscpy(pm_name, pm_ops ? pm_ops->name : "kernel", MPTCP_PM_NAME_MAX);
+	rcu_read_unlock();
 
 	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
 	if (write && ret == 0) {
-		ret = mptcp_set_path_manager(*path_manager, pm_name);
+		ret = mptcp_set_path_manager(pernet, pm_name);
 		if (ret == 0) {
 			u8 pm_type = __MPTCP_PM_TYPE_NR;
 
@@ -267,7 +275,7 @@ static int proc_pm_type(const struct ctl_table *ctl, int write,
 			pm_name = "kernel";
 		else if (pm_type == MPTCP_PM_TYPE_USERSPACE)
 			pm_name = "userspace";
-		mptcp_set_path_manager(pernet->path_manager, pm_name);
+		mptcp_set_path_manager(pernet, pm_name);
 	}
 
 	return ret;
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index ba7c6f80a183..09f99bcd827c 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -1204,7 +1204,7 @@ void mptcp_pm_destroy(struct mptcp_sock *msk)
 void mptcp_pm_data_reset(struct mptcp_sock *msk)
 {
 	const struct net *net = sock_net((struct sock *)msk);
-	const char *pm_name = mptcp_get_path_manager(net);
+	char pm_name[MPTCP_PM_NAME_MAX];
 	u8 pm_type = mptcp_get_pm_type(net);
 	struct mptcp_pm_data *pm = &msk->pm;
 
@@ -1213,6 +1213,7 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
 	pm->rm_list_rx.nr = 0;
 	WRITE_ONCE(pm->pm_type, pm_type);
 
+	mptcp_get_path_manager(net, pm_name);
 	rcu_read_lock();
 	mptcp_pm_ops_init(msk, pm_name);
 	rcu_read_unlock();
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index af79b3450ab7..99f447f2808f 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -802,7 +802,7 @@ int mptcp_allow_join_id0(const struct net *net);
 unsigned int mptcp_stale_loss_cnt(const struct net *net);
 unsigned int mptcp_close_timeout(const struct sock *sk);
 int mptcp_get_pm_type(const struct net *net);
-const char *mptcp_get_path_manager(const struct net *net);
+void mptcp_get_path_manager(const struct net *net, char *name);
 void mptcp_get_scheduler(const struct net *net, char *name);
 unsigned int mptcp_add_addr_v6_port_drop_ts(const struct net *net);
 
@@ -1153,7 +1153,6 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk,
 			   bool echo);
 int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_list);
 
-/* the default path manager, used in mptcp_pm_unregister */
 extern struct mptcp_pm_ops mptcp_pm_kernel;
 extern struct mptcp_sched_ops mptcp_sched_default;
 
-- 
2.43.0

Re: [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing module reference counting when setting the path manager sysctl leads to a Use-After-Free.
- [High] Missing module reference counting when setting the scheduler sysctl leads to a Use-After-Free.
--

--- Patch [2]: [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically ---
commit c42ca00640178ef2df1ab9f51d1f65a9b722ec86
Author: Gang Yan <yangang@kylinos.cn>

mptcp: pm: change path_manager sysctl atomically

The per-netns path manager name is stored as an inline char[] buffer and
updated via strscpy() from the sysctl handler; a concurrent reader can
observe a half-written name. This patch replaces the string with a
pointer to the immutable struct mptcp_pm_ops.

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

> Following the tcp_congestion_control() model (and the scheduler change in
> the previous patch), store a pointer to the immutable struct mptcp_pm_ops
> instead of the name string:

[Severity: High]
Since this patch follows the scheduler change in the previous patch, does
mptcp_set_scheduler() have the same module reference counting issue?

If mptcp_set_scheduler() assigns pernet->scheduler without acquiring a
module reference, and the BPF map is unloaded, could a new MPTCP socket
creation trigger mptcp_init_sock() which calls mptcp_get_scheduler(),
causing a Use-After-Free when it dereferences the freed sched pointer?

> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index 479b31eb30079..c0481b09c1a17 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c
[ ... ]
> @@ -85,9 +85,14 @@ int mptcp_get_pm_type(const struct net *net)
>  	return mptcp_get_pernet(net)->pm_type;
>  }
>  
> -const char *mptcp_get_path_manager(const struct net *net)
> +void mptcp_get_path_manager(const struct net *net, char *name)
>  {
> -	return mptcp_get_pernet(net)->path_manager;
> +	struct mptcp_pm_ops *pm_ops;
> +
> +	rcu_read_lock();
> +	pm_ops = rcu_dereference(mptcp_get_pernet(net)->path_manager);
> +	strscpy(name, pm_ops ? pm_ops->name : "kernel", MPTCP_PM_NAME_MAX);

[Severity: High]
If the module was unloaded and the pointer was not pinned, would this
dereference of pm_ops->name result in a Use-After-Free?

> +	rcu_read_unlock();
>  }
[ ... ]
> @@ -201,7 +206,7 @@ static int proc_blackhole_detect_timeout(const struct ctl_table *table,
[ ... ]
>  	rcu_read_lock();
>  	pm_ops = mptcp_pm_find(name);
>  	if (pm_ops)
> -		strscpy(path_manager, name, MPTCP_PM_NAME_MAX);
> +		xchg(&pernet->path_manager, pm_ops);

[Severity: High]
Does this assignment need to pin the module (e.g., via bpf_try_module_get())
before saving the pointer?

If a user with CAP_NET_ADMIN sets a BPF struct_ops map as the default path
manager, and then unloads the BPF program, the map is deleted and the
struct mptcp_pm_ops memory is freed.

Could this leave a dangling pointer in pernet->path_manager that causes the
Use-After-Free mentioned above when mptcp_get_path_manager() is subsequently
called?

>  	else
>  		ret = -ENOENT;
>  	rcu_read_unlock();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818094825.48446-1-gang.yan@linux.dev?part=2

Re: [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically

[gang.yan]

August 18, 2026 at 6:00 PM, sashiko-bot@kernel.org mailto:sashiko-bot@kernel.org  wrote:


> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Missing module reference counting when setting the path manager sysctl leads to a Use-After-Free.
> - [High] Missing module reference counting when setting the scheduler sysctl leads to a Use-After-Free.
> --

Hi, Matt

As described in [1], it is safe for now.

Do you have some suggestions here? Or we can ignore these.

Thanks
Gang

> 
> --- Patch [2]: [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically ---
> commit c42ca00640178ef2df1ab9f51d1f65a9b722ec86
> Author: Gang Yan <yangang@kylinos.cn>
> 
> mptcp: pm: change path_manager sysctl atomically
> 
> The per-netns path manager name is stored as an inline char[] buffer and
> updated via strscpy() from the sysctl handler; a concurrent reader can
> observe a half-written name. This patch replaces the string with a
> pointer to the immutable struct mptcp_pm_ops.
> 
> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/626
> 
> > 
> > Following the tcp_congestion_control() model (and the scheduler change in
> >  the previous patch), store a pointer to the immutable struct mptcp_pm_ops
> >  instead of the name string:
> > 
> [Severity: High]
> Since this patch follows the scheduler change in the previous patch, does
> mptcp_set_scheduler() have the same module reference counting issue?
> 
> If mptcp_set_scheduler() assigns pernet->scheduler without acquiring a
> module reference, and the BPF map is unloaded, could a new MPTCP socket
> creation trigger mptcp_init_sock() which calls mptcp_get_scheduler(),
> causing a Use-After-Free when it dereferences the freed sched pointer?
> 
> > 
> > diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> >  index 479b31eb30079..c0481b09c1a17 100644
> >  --- a/net/mptcp/ctrl.c
> >  +++ b/net/mptcp/ctrl.c
> > 
> [ ... ]
> 
> > 
> > @@ -85,9 +85,14 @@ int mptcp_get_pm_type(const struct net *net)
> >  return mptcp_get_pernet(net)->pm_type;
> >  }
> >  
> >  -const char *mptcp_get_path_manager(const struct net *net)
> >  +void mptcp_get_path_manager(const struct net *net, char *name)
> >  {
> >  - return mptcp_get_pernet(net)->path_manager;
> >  + struct mptcp_pm_ops *pm_ops;
> >  +
> >  + rcu_read_lock();
> >  + pm_ops = rcu_dereference(mptcp_get_pernet(net)->path_manager);
> >  + strscpy(name, pm_ops ? pm_ops->name : "kernel", MPTCP_PM_NAME_MAX);
> > 
> [Severity: High]
> If the module was unloaded and the pointer was not pinned, would this
> dereference of pm_ops->name result in a Use-After-Free?
> 
> > 
> > + rcu_read_unlock();
> >  }
> > 
> [ ... ]
> 
> > 
> > @@ -201,7 +206,7 @@ static int proc_blackhole_detect_timeout(const struct ctl_table *table,
> > 
> [ ... ]
> 
> > 
> > rcu_read_lock();
> >  pm_ops = mptcp_pm_find(name);
> >  if (pm_ops)
> >  - strscpy(path_manager, name, MPTCP_PM_NAME_MAX);
> >  + xchg(&pernet->path_manager, pm_ops);
> > 
> [Severity: High]
> Does this assignment need to pin the module (e.g., via bpf_try_module_get())
> before saving the pointer?
> 
> If a user with CAP_NET_ADMIN sets a BPF struct_ops map as the default path
> manager, and then unloads the BPF program, the map is deleted and the
> struct mptcp_pm_ops memory is freed.
> 
> Could this leave a dangling pointer in pernet->path_manager that causes the
> Use-After-Free mentioned above when mptcp_get_path_manager() is subsequently
> called?
> 
> > 
> > else
> >  ret = -ENOENT;
> >  rcu_read_unlock();
> > 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260818094825.48446-1-gang.yan@linux.dev?part=2
>

Re: [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically

[Matthieu Baerts]

Hi Gang,

On 18/08/2026 14:57, gang.yan@linux.dev wrote:
> August 18, 2026 at 6:00 PM, sashiko-bot@kernel.org mailto:sashiko-bot@kernel.org  wrote:
> 
> 
>>
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>> - [High] Missing module reference counting when setting the path manager sysctl leads to a Use-After-Free.
>> - [High] Missing module reference counting when setting the scheduler sysctl leads to a Use-After-Free.
>> --
> 
> Hi, Matt
> 
> As described in [1], it is safe for now.
> 
> Do you have some suggestions here? Or we can ignore these.

We can indeed ignore.

Do you mind adding a comment in the commit message in your v3 to tell
Sashiko that the module reference counting is done in patch X of this
series? Just to see if it helps Sashiko to avoid such messages.

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

[PATCH mptcp-next v2 3/5] mptcp: use READ_ONCE() over sysctls

[Gang Yan]

From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>

To avoid KCSAN issues.

This patch is in theory for -net, and will need to be split in multiple
patches, with different Fixes tags. But I prefer to wait for Eric's
patches, as I noticed he already started to modify mptcp_is_enabled:

  https://lore.kernel.org/CANn89iLdwhhwLyO6zRjWMEY3t9g60ZE8ZhOVx33ucg_uRETbmQ@mail.gmail.com

Still, keeping this patch in this series, not to forget about it.

Also write the pm_type field derived from the path manager name with
WRITE_ONCE() in proc_path_manager(), to pair with the READ_ONCE() on
mptcp_get_pm_type() added by this patch.

Reported-by: Eric Dumazet <edumazet@google.com>
Closes: https://lore.kernel.org/CANn89iL=os-60kDKqMDdyiXuPF5CG=eejS0vmthwpDGXz_Bp8A@mail.gmail.com
Co-developed-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/ctrl.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index c0481b09c1a1..733706f06f1b 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -50,39 +50,39 @@ static struct mptcp_pernet *mptcp_get_pernet(const struct net *net)
 
 int mptcp_is_enabled(const struct net *net)
 {
-	return mptcp_get_pernet(net)->mptcp_enabled;
+	return READ_ONCE(mptcp_get_pernet(net)->mptcp_enabled);
 }
 
 unsigned int mptcp_get_add_addr_timeout(const struct net *net)
 {
-	return mptcp_get_pernet(net)->add_addr_timeout;
+	return READ_ONCE(mptcp_get_pernet(net)->add_addr_timeout);
 }
 
 int mptcp_is_checksum_enabled(const struct net *net)
 {
-	return mptcp_get_pernet(net)->checksum_enabled;
+	return READ_ONCE(mptcp_get_pernet(net)->checksum_enabled);
 }
 
 int mptcp_allow_join_id0(const struct net *net)
 {
-	return mptcp_get_pernet(net)->allow_join_initial_addr_port;
+	return READ_ONCE(mptcp_get_pernet(net)->allow_join_initial_addr_port);
 }
 
 unsigned int mptcp_stale_loss_cnt(const struct net *net)
 {
-	return mptcp_get_pernet(net)->stale_loss_cnt;
+	return READ_ONCE(mptcp_get_pernet(net)->stale_loss_cnt);
 }
 
 unsigned int mptcp_close_timeout(const struct sock *sk)
 {
 	if (sock_flag(sk, SOCK_DEAD))
 		return TCP_TIMEWAIT_LEN;
-	return mptcp_get_pernet(sock_net(sk))->close_timeout;
+	return READ_ONCE(mptcp_get_pernet(sock_net(sk))->close_timeout);
 }
 
 int mptcp_get_pm_type(const struct net *net)
 {
-	return mptcp_get_pernet(net)->pm_type;
+	return READ_ONCE(mptcp_get_pernet(net)->pm_type);
 }
 
 void mptcp_get_path_manager(const struct net *net, char *name)
@@ -251,7 +251,7 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
 				pm_type = MPTCP_PM_TYPE_KERNEL;
 			else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
 				pm_type = MPTCP_PM_TYPE_USERSPACE;
-			pernet->pm_type = pm_type;
+			WRITE_ONCE(pernet->pm_type, pm_type);
 		}
 	}
 
@@ -572,7 +572,7 @@ void mptcp_active_detect_blackhole(struct sock *ssk, bool expired)
 
 	net = sock_net(ssk);
 	timeouts = inet_csk(ssk)->icsk_retransmits;
-	to_max = mptcp_get_pernet(net)->syn_retrans_before_tcp_fallback;
+	to_max = READ_ONCE(mptcp_get_pernet(net)->syn_retrans_before_tcp_fallback);
 
 	if (timeouts == to_max || (timeouts < to_max && expired)) {
 		subflow->mpc_drop = 1;
-- 
2.43.0

Re: [PATCH mptcp-next v2 3/5] mptcp: use READ_ONCE() over sysctls

[sashiko-bot]

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

Pre-existing issues:
- [High] Concurrent sysctl writes or sysctl updates racing with socket creation can leave the socket's `pm_type` and `ops` mismatched, breaking MPTCP path management.
--

--- Patch [3]: [PATCH mptcp-next v2 3/5] mptcp: use READ_ONCE() over sysctls ---
commit 77c9e4aa4cdd5f6c57cc4cab253d46fd0cf2dfb2
Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>

mptcp: use READ_ONCE() over sysctls

This commit adds READ_ONCE() and WRITE_ONCE() macros around sysctl
variables in the MPTCP networking code to avoid KCSAN issues. It also
ensures the pm_type is written with WRITE_ONCE() in proc_path_manager().

Closes: https://lore.kernel.org/CANn89iL=os-60kDKqMDdyiXuPF5CG=eejS0vmthwpDGXz_Bp8A@mail.gmail.com

> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index c0481b09c1a17..733706f06f1b1 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c

[ ... ]

> @@ -251,7 +251,7 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
>  				pm_type = MPTCP_PM_TYPE_KERNEL;
>  			else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
>  				pm_type = MPTCP_PM_TYPE_USERSPACE;
> -			pernet->pm_type = pm_type;
> +			WRITE_ONCE(pernet->pm_type, pm_type);

[Severity: High]
This is a pre-existing issue, but can this sequential update race with socket
creation and leave the socket's pm_type and ops mismatched?

If an administrator writes to the sysctl while a new socket is being created,
could mptcp_pm_data_reset() in net/mptcp/pm.c interleave like this:

CPU1 (sysctl write)
proc_path_manager()
    mptcp_set_path_manager(pernet, pm_name); // Updates pernet->path_manager

CPU2 (socket creation)
mptcp_pm_data_reset()
    pm_type = mptcp_get_pm_type(net);        // Reads old pm_type
    ...
    mptcp_get_path_manager(net, pm_name);    // Reads new path_manager

CPU1 (sysctl write resumes)
proc_path_manager()
    WRITE_ONCE(pernet->pm_type, pm_type);    // Updates pernet->pm_type

Would this leave the new socket corrupted with a contradictory path manager
state, breaking MPTCP path management logic?

>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818094825.48446-1-gang.yan@linux.dev?part=3

Re: [PATCH mptcp-next v2 3/5] mptcp: use READ_ONCE() over sysctls

[gang.yan]

August 18, 2026 at 6:01 PM, sashiko-bot@kernel.org mailto:sashiko-bot@kernel.org  wrote:


> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Concurrent sysctl writes or sysctl updates racing with socket creation can leave the socket's `pm_type` and `ops` mismatched, breaking MPTCP path management.
> --
> 
> --- Patch [3]: [PATCH mptcp-next v2 3/5] mptcp: use READ_ONCE() over sysctls ---
> commit 77c9e4aa4cdd5f6c57cc4cab253d46fd0cf2dfb2
> Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> mptcp: use READ_ONCE() over sysctls
> 
> This commit adds READ_ONCE() and WRITE_ONCE() macros around sysctl
> variables in the MPTCP networking code to avoid KCSAN issues. It also
> ensures the pm_type is written with WRITE_ONCE() in proc_path_manager().
> 
> Closes: https://lore.kernel.org/CANn89iL=os-60kDKqMDdyiXuPF5CG=eejS0vmthwpDGXz_Bp8A@mail.gmail.com
> 
> > 
> > diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> >  index c0481b09c1a17..733706f06f1b1 100644
> >  --- a/net/mptcp/ctrl.c
> >  +++ b/net/mptcp/ctrl.c
> > 
> [ ... ]
> 
> > 
> > @@ -251,7 +251,7 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
> >  pm_type = MPTCP_PM_TYPE_KERNEL;
> >  else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
> >  pm_type = MPTCP_PM_TYPE_USERSPACE;
> >  - pernet->pm_type = pm_type;
> >  + WRITE_ONCE(pernet->pm_type, pm_type);
> > 
> [Severity: High]
> This is a pre-existing issue, but can this sequential update race with socket
> creation and leave the socket's pm_type and ops mismatched?
> 
> If an administrator writes to the sysctl while a new socket is being created,
> could mptcp_pm_data_reset() in net/mptcp/pm.c interleave like this:
> 
> CPU1 (sysctl write)
> proc_path_manager()
>  mptcp_set_path_manager(pernet, pm_name); // Updates pernet->path_manager
> 
> CPU2 (socket creation)
> mptcp_pm_data_reset()
>  pm_type = mptcp_get_pm_type(net); // Reads old pm_type
>  ...
>  mptcp_get_path_manager(net, pm_name); // Reads new path_manager
> 
> CPU1 (sysctl write resumes)
> proc_path_manager()
>  WRITE_ONCE(pernet->pm_type, pm_type); // Updates pernet->pm_type
> 
> Would this leave the new socket corrupted with a contradictory path manager
> state, breaking MPTCP path management logic?

Hi,

I think that may happen. Maybe need anothor patch to fix this.

When I have time, I'll look this.

Thanks
Gang


> 
> > 
> > }
> >  }
> > 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260818094825.48446-1-gang.yan@linux.dev?part=3
>

Re: [PATCH mptcp-next v2 3/5] mptcp: use READ_ONCE() over sysctls

[Matthieu Baerts]

Hi Gang,

On 18/08/2026 15:02, gang.yan@linux.dev wrote:
> August 18, 2026 at 6:01 PM, sashiko-bot@kernel.org mailto:sashiko-bot@kernel.org  wrote:
> 
> 
>>
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>
>> Pre-existing issues:
>> - [High] Concurrent sysctl writes or sysctl updates racing with socket creation can leave the socket's `pm_type` and `ops` mismatched, breaking MPTCP path management.
>> --
>>
>> --- Patch [3]: [PATCH mptcp-next v2 3/5] mptcp: use READ_ONCE() over sysctls ---
>> commit 77c9e4aa4cdd5f6c57cc4cab253d46fd0cf2dfb2
>> Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>
>> mptcp: use READ_ONCE() over sysctls
>>
>> This commit adds READ_ONCE() and WRITE_ONCE() macros around sysctl
>> variables in the MPTCP networking code to avoid KCSAN issues. It also
>> ensures the pm_type is written with WRITE_ONCE() in proc_path_manager().
>>
>> Closes: https://lore.kernel.org/CANn89iL=os-60kDKqMDdyiXuPF5CG=eejS0vmthwpDGXz_Bp8A@mail.gmail.com
>>
>>>
>>> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
>>>  index c0481b09c1a17..733706f06f1b1 100644
>>>  --- a/net/mptcp/ctrl.c
>>>  +++ b/net/mptcp/ctrl.c
>>>
>> [ ... ]
>>
>>>
>>> @@ -251,7 +251,7 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
>>>  pm_type = MPTCP_PM_TYPE_KERNEL;
>>>  else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
>>>  pm_type = MPTCP_PM_TYPE_USERSPACE;
>>>  - pernet->pm_type = pm_type;
>>>  + WRITE_ONCE(pernet->pm_type, pm_type);
>>>
>> [Severity: High]
>> This is a pre-existing issue, but can this sequential update race with socket
>> creation and leave the socket's pm_type and ops mismatched?
>>
>> If an administrator writes to the sysctl while a new socket is being created,
>> could mptcp_pm_data_reset() in net/mptcp/pm.c interleave like this:
>>
>> CPU1 (sysctl write)
>> proc_path_manager()
>>  mptcp_set_path_manager(pernet, pm_name); // Updates pernet->path_manager
>>
>> CPU2 (socket creation)
>> mptcp_pm_data_reset()
>>  pm_type = mptcp_get_pm_type(net); // Reads old pm_type
>>  ...
>>  mptcp_get_path_manager(net, pm_name); // Reads new path_manager
>>
>> CPU1 (sysctl write resumes)
>> proc_path_manager()
>>  WRITE_ONCE(pernet->pm_type, pm_type); // Updates pernet->pm_type
>>
>> Would this leave the new socket corrupted with a contradictory path manager
>> state, breaking MPTCP path management logic?
> 
> Hi,
> 
> I think that may happen. Maybe need anothor patch to fix this.
> 
> When I have time, I'll look this.
Indeed, that's for another patch, probably a fix for -net.

But it feels like you would need a lock to handle that properly. If it
starts to be too complex, we could also say that the net.mptcp.pm_type
sysctl knob is deprecated, and that's a known issue. In this case, it
might be good to add a comment above this WRITE_ONCE(). Then, this could
be done in a dedicated patch I think to explain why. This "mptcp: use
READ_ONCE() over sysctls" would then be only about ... READ_ONCE() :)

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

[PATCH mptcp-next v2 4/5] Squash to "mptcp: pm: init and release mptcp_pm_ops"

[Gang Yan]

From: Gang Yan <yangang@kylinos.cn>

This commit introduces the mptcp_pm_ops lifetime handling on sockets
(mptcp_pm_ops_init/release taking a module reference), and would then
be the first one whose per-net path managers can be unloaded while a
pernet still stores them. Extend the same reference handling to the
pernet level:

  - mptcp_pernet_set_defaults() pins &mptcp_pm_kernel;
  - mptcp_set_path_manager() takes a reference on the new ops and
    releases the one held on the ops it replaces;
  - mptcp_net_exit() releases the last reference.

Depends on the sysctl patches earlier in this series.

Assisted-by: Claude:GLM5.2
Co-developed-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 net/mptcp/ctrl.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 733706f06f1b..32268aa26097 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -124,7 +124,9 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
 	pernet->pm_type = MPTCP_PM_TYPE_KERNEL;
 
 	RCU_INIT_POINTER(pernet->scheduler, &mptcp_sched_default);
-	RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);
+
+	if (bpf_try_module_get(&mptcp_pm_kernel, mptcp_pm_kernel.owner))
+		RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);
 
 	pernet->add_addr_v6_port_drop_ts = 1;
 }
@@ -208,15 +210,22 @@ static int proc_blackhole_detect_timeout(const struct ctl_table *table,
 
 static int mptcp_set_path_manager(struct mptcp_pernet *pernet, const char *name)
 {
-	struct mptcp_pm_ops *pm_ops;
+	struct mptcp_pm_ops *pm_ops, *prev;
 	int ret = 0;
 
 	rcu_read_lock();
 	pm_ops = mptcp_pm_find(name);
-	if (pm_ops)
-		xchg(&pernet->path_manager, pm_ops);
-	else
+	if (pm_ops) {
+		if (bpf_try_module_get(pm_ops, pm_ops->owner)) {
+			prev = xchg(&pernet->path_manager, pm_ops);
+			if (prev)
+				bpf_module_put(prev, prev->owner);
+		} else {
+			ret = -EBUSY;
+		}
+	} else {
 		ret = -ENOENT;
+	}
 	rcu_read_unlock();
 
 	return ret;
@@ -594,8 +603,13 @@ static int __net_init mptcp_net_init(struct net *net)
 static void __net_exit mptcp_net_exit(struct net *net)
 {
 	struct mptcp_pernet *pernet = mptcp_get_pernet(net);
+	struct mptcp_pm_ops *pm;
 
 	mptcp_pernet_del_table(pernet);
+
+	pm = rcu_dereference_protected(pernet->path_manager, true);
+	if (pm)
+		bpf_module_put(pm, pm->owner);
 }
 
 static struct pernet_operations mptcp_pernet_ops = {
-- 
2.43.0

Re: [PATCH mptcp-next v2 4/5] Squash to "mptcp: pm: init and release mptcp_pm_ops"

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Resource Leak on Error Path in `mptcp_net_init`
--

--- Patch [4]: [PATCH mptcp-next v2 4/5] Squash to "mptcp: pm: init and release mptcp_pm_ops" ---
commit b8f493287a2fef63b437fe19fc22666b77c1c08b
Author: Gang Yan <yangang@kylinos.cn>

Squash to "mptcp: pm: init and release mptcp_pm_ops"

This commit introduces lifetime handling for mptcp_pm_ops at the pernet
level. It pins the default mptcp_pm_kernel by taking a module reference
during pernet defaults initialization and releases it during network
namespace exit.

> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index 733706f06f1b1..32268aa260976 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c
> @@ -124,7 +124,9 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
>  	pernet->pm_type = MPTCP_PM_TYPE_KERNEL;
>  
>  	RCU_INIT_POINTER(pernet->scheduler, &mptcp_sched_default);
> -	RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);
> +
> +	if (bpf_try_module_get(&mptcp_pm_kernel, mptcp_pm_kernel.owner))
> +		RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);

[Severity: Medium]
Does this introduce a module reference leak regression on the error path
during network namespace initialization?

When mptcp_pernet_set_defaults() takes this module reference, it is called
from mptcp_net_init() right before mptcp_pernet_new_table():

mptcp_net_init() {
    mptcp_pernet_set_defaults(pernet);
    return mptcp_pernet_new_table(net, pernet);
}

If mptcp_pernet_new_table() fails and returns an error, the error is
propagated without releasing the module reference acquired here. Because the
initialization failed, the core network namespace code will not call the
corresponding exit function mptcp_net_exit() to release it.

Should mptcp_net_init() clean up the reference if mptcp_pernet_new_table()
fails?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818094825.48446-1-gang.yan@linux.dev?part=4

Re: [PATCH mptcp-next v2 4/5] Squash to "mptcp: pm: init and release mptcp_pm_ops"

[gang.yan]

August 18, 2026 at 6:03 PM, sashiko-bot@kernel.org mailto:sashiko-bot@kernel.org  wrote:


> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Resource Leak on Error Path in `mptcp_net_init`
> --
> 
> --- Patch [4]: [PATCH mptcp-next v2 4/5] Squash to "mptcp: pm: init and release mptcp_pm_ops" ---
> commit b8f493287a2fef63b437fe19fc22666b77c1c08b
> Author: Gang Yan <yangang@kylinos.cn>
> 
> Squash to "mptcp: pm: init and release mptcp_pm_ops"
> 
> This commit introduces lifetime handling for mptcp_pm_ops at the pernet
> level. It pins the default mptcp_pm_kernel by taking a module reference
> during pernet defaults initialization and releases it during network
> namespace exit.
> 
> > 
> > diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> >  index 733706f06f1b1..32268aa260976 100644
> >  --- a/net/mptcp/ctrl.c
> >  +++ b/net/mptcp/ctrl.c
> >  @@ -124,7 +124,9 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
> >  pernet->pm_type = MPTCP_PM_TYPE_KERNEL;
> >  
> >  RCU_INIT_POINTER(pernet->scheduler, &mptcp_sched_default);
> >  - RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);
> >  +
> >  + if (bpf_try_module_get(&mptcp_pm_kernel, mptcp_pm_kernel.owner))
> >  + RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);
> > 
> [Severity: Medium]
> Does this introduce a module reference leak regression on the error path
> during network namespace initialization?
> 
> When mptcp_pernet_set_defaults() takes this module reference, it is called
> from mptcp_net_init() right before mptcp_pernet_new_table():
> 
> mptcp_net_init() {
>  mptcp_pernet_set_defaults(pernet);
>  return mptcp_pernet_new_table(net, pernet);
> }
> 
> If mptcp_pernet_new_table() fails and returns an error, the error is
> propagated without releasing the module reference acquired here. Because the
> initialization failed, the core network namespace code will not call the
> corresponding exit function mptcp_net_exit() to release it.
> 
> Should mptcp_net_init() clean up the reference if mptcp_pernet_new_table()
> fails?
> 

Good catch.

Maybe check the return val of mptcp_pernet_new_table could handle this. It can
be done in v3.

Thanks
Gang


> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260818094825.48446-1-gang.yan@linux.dev?part=4
>

[PATCH mptcp-next v2 5/5] Squash to "bpf: Add mptcp packet scheduler struct_ops"

[Gang Yan]

From: Gang Yan <yangang@kylinos.cn>

The pernet sysctl patches in this series swap the scheduler pointer
with xchg() without taking any module reference: this is fine while
ops can only be registered from built-in code.

This commit is what makes BPF schedulers registerable, i.e. the first
one whose ops can be unloaded while a pernet still stores them. Take
the reference accordingly:

  - mptcp_pernet_set_defaults() pins &mptcp_sched_default, mirroring
    tcp_sk_init() pinning &tcp_reno;
  - mptcp_set_scheduler() takes a reference on the new ops and
    releases the one held on the ops it replaces, mirroring
    tcp_set_default_congestion_control();
  - mptcp_net_exit() releases the last reference, mirroring
    tcp_sk_exit().

Assisted-by: Claude:GLM5.2
Co-developed-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 net/mptcp/ctrl.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 32268aa26097..309bbb2a4eb0 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -123,7 +123,8 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
 	pernet->stale_loss_cnt = 4;
 	pernet->pm_type = MPTCP_PM_TYPE_KERNEL;
 
-	RCU_INIT_POINTER(pernet->scheduler, &mptcp_sched_default);
+	if (bpf_try_module_get(&mptcp_sched_default, mptcp_sched_default.owner))
+		RCU_INIT_POINTER(pernet->scheduler, &mptcp_sched_default);
 
 	if (bpf_try_module_get(&mptcp_pm_kernel, mptcp_pm_kernel.owner))
 		RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);
@@ -134,15 +135,22 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
 #ifdef CONFIG_SYSCTL
 static int mptcp_set_scheduler(struct mptcp_pernet *pernet, const char *name)
 {
-	struct mptcp_sched_ops *sched;
+	struct mptcp_sched_ops *sched, *prev;
 	int ret = 0;
 
 	rcu_read_lock();
 	sched = mptcp_sched_find(name);
-	if (sched)
-		xchg(&pernet->scheduler, sched);
-	else
+	if (sched) {
+		if (bpf_try_module_get(sched, sched->owner)) {
+			prev = xchg(&pernet->scheduler, sched);
+			if (prev)
+				bpf_module_put(prev, prev->owner);
+		} else {
+			ret = -EBUSY;
+		}
+	} else {
 		ret = -ENOENT;
+	}
 	rcu_read_unlock();
 
 	return ret;
@@ -603,10 +611,15 @@ static int __net_init mptcp_net_init(struct net *net)
 static void __net_exit mptcp_net_exit(struct net *net)
 {
 	struct mptcp_pernet *pernet = mptcp_get_pernet(net);
+	struct mptcp_sched_ops *sched;
 	struct mptcp_pm_ops *pm;
 
 	mptcp_pernet_del_table(pernet);
 
+	sched = rcu_dereference_protected(pernet->scheduler, true);
+	if (sched)
+		bpf_module_put(sched, sched->owner);
+
 	pm = rcu_dereference_protected(pernet->path_manager, true);
 	if (pm)
 		bpf_module_put(pm, pm->owner);
-- 
2.43.0

Re: [PATCH mptcp-next v2 0/5] mptcp: avoid data-races around the sysctls

[MPTCP CI]

Hi Gang,

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

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


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)