* [PATCH mptcp-next v2 0/5] mptcp: avoid data-races around the sysctls
@ 2026-08-18 9:39 Gang Yan
2026-08-18 9:39 ` [PATCH mptcp-next v2 1/5] mptcp: sched: change scheduler sysctl atomically Gang Yan
2026-08-18 9:39 ` [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager " Gang Yan
0 siblings, 2 replies; 8+ messages in thread
From: Gang Yan @ 2026-08-18 9:39 UTC (permalink / raw)
To: mptcp
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH mptcp-next v2 1/5] mptcp: sched: change scheduler sysctl atomically
2026-08-18 9:39 [PATCH mptcp-next v2 0/5] mptcp: avoid data-races around the sysctls Gang Yan
@ 2026-08-18 9:39 ` Gang Yan
2026-08-18 9:39 ` [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager " Gang Yan
1 sibling, 0 replies; 8+ messages in thread
From: Gang Yan @ 2026-08-18 9:39 UTC (permalink / raw)
To: mptcp
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically
2026-08-18 9:39 [PATCH mptcp-next v2 0/5] mptcp: avoid data-races around the sysctls Gang Yan
2026-08-18 9:39 ` [PATCH mptcp-next v2 1/5] mptcp: sched: change scheduler sysctl atomically Gang Yan
@ 2026-08-18 9:39 ` Gang Yan
2026-08-18 9:48 ` gang.yan
1 sibling, 1 reply; 8+ messages in thread
From: Gang Yan @ 2026-08-18 9:39 UTC (permalink / raw)
To: mptcp
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically
2026-08-18 9:39 ` [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager " Gang Yan
@ 2026-08-18 9:48 ` gang.yan
0 siblings, 0 replies; 8+ messages in thread
From: gang.yan @ 2026-08-18 9:48 UTC (permalink / raw)
To: mptcp
August 18, 2026 at 5:39 PM, "Gang Yan" <gang.yan@linux.dev mailto:gang.yan@linux.dev?to=%22Gang%20Yan%22%20%3Cgang.yan%40linux.dev%3E > wrote:
Hi
Sorry for that. It seems that the rest of this series is not sent successfully
due to the bad network. I'll resend it.
Please ignore this noise.
Thanks
Gang
>
> 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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically
2026-08-18 9:48 [PATCH mptcp-next v2 0/5] mptcp: avoid data-races around the sysctls Gang Yan
@ 2026-08-18 9:48 ` Gang Yan
2026-08-18 10:00 ` sashiko-bot
0 siblings, 1 reply; 8+ messages in thread
From: Gang Yan @ 2026-08-18 9:48 UTC (permalink / raw)
To: mptcp
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically
2026-08-18 9:48 ` [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically Gang Yan
@ 2026-08-18 10:00 ` sashiko-bot
2026-08-18 12:57 ` gang.yan
0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-08-18 10:00 UTC (permalink / raw)
To: Gang Yan; +Cc: mptcp
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically
2026-08-18 10:00 ` sashiko-bot
@ 2026-08-18 12:57 ` gang.yan
2026-08-18 15:52 ` Matthieu Baerts
0 siblings, 1 reply; 8+ messages in thread
From: gang.yan @ 2026-08-18 12:57 UTC (permalink / raw)
To: sashiko-reviews; +Cc: mptcp
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically
2026-08-18 12:57 ` gang.yan
@ 2026-08-18 15:52 ` Matthieu Baerts
0 siblings, 0 replies; 8+ messages in thread
From: Matthieu Baerts @ 2026-08-18 15:52 UTC (permalink / raw)
To: gang.yan, sashiko-reviews; +Cc: mptcp
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-18 15:52 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 9:39 [PATCH mptcp-next v2 0/5] mptcp: avoid data-races around the sysctls Gang Yan
2026-08-18 9:39 ` [PATCH mptcp-next v2 1/5] mptcp: sched: change scheduler sysctl atomically Gang Yan
2026-08-18 9:39 ` [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager " Gang Yan
2026-08-18 9:48 ` gang.yan
-- strict thread matches above, loose matches on Subject: below --
2026-08-18 9:48 [PATCH mptcp-next v2 0/5] mptcp: avoid data-races around the sysctls Gang Yan
2026-08-18 9:48 ` [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically Gang Yan
2026-08-18 10:00 ` sashiko-bot
2026-08-18 12:57 ` gang.yan
2026-08-18 15:52 ` Matthieu Baerts
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox