* [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 1/5] mptcp: sched: change scheduler 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 9:59 ` 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 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
* Re: [PATCH mptcp-next v2 1/5] mptcp: sched: change scheduler sysctl atomically
2026-08-18 9:48 ` [PATCH mptcp-next v2 1/5] mptcp: sched: change scheduler sysctl atomically Gang Yan
@ 2026-08-18 9:59 ` sashiko-bot
2026-08-18 12:56 ` gang.yan
0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-08-18 9:59 UTC (permalink / raw)
To: Gang Yan; +Cc: mptcp
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH mptcp-next v2 1/5] mptcp: sched: change scheduler sysctl atomically
2026-08-18 9:59 ` sashiko-bot
@ 2026-08-18 12:56 ` gang.yan
2026-08-18 15:50 ` Matthieu Baerts
0 siblings, 1 reply; 8+ messages in thread
From: gang.yan @ 2026-08-18 12:56 UTC (permalink / raw)
To: sashiko-reviews; +Cc: mptcp
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH mptcp-next v2 1/5] mptcp: sched: change scheduler sysctl atomically
2026-08-18 12:56 ` gang.yan
@ 2026-08-18 15:50 ` Matthieu Baerts
0 siblings, 0 replies; 8+ messages in thread
From: Matthieu Baerts @ 2026-08-18 15:50 UTC (permalink / raw)
To: gang.yan, sashiko-reviews; +Cc: mptcp
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-18 15:50 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 1/5] mptcp: sched: change scheduler sysctl atomically Gang Yan
2026-08-18 9:59 ` sashiko-bot
2026-08-18 12:56 ` gang.yan
2026-08-18 15:50 ` Matthieu Baerts
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.