* [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 0/5] mptcp: avoid data-races around the sysctls
@ 2026-08-18 9:48 Gang Yan
2026-08-18 9:48 ` [PATCH mptcp-next v2 2/5] mptcp: pm: change path_manager sysctl atomically Gang Yan
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>
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 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 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.