* [PATCH mptcp-next v3 1/5] mptcp: sched: change scheduler sysctl atomically
2026-08-19 12:56 [PATCH mptcp-next v3 0/5] mptcp: avoid data-races around the sysctls Gang Yan
@ 2026-08-19 12:56 ` Gang Yan
2026-08-19 12:56 ` [PATCH mptcp-next v3 2/5] mptcp: pm: change path_manager " Gang Yan
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Gang Yan @ 2026-08-19 12:56 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. A pointer
store is a single atomic word, so readers always observe a consistent
value, and mptcp_get_scheduler() can now return the ops directly
instead of going through mptcp_sched_find() again.
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 | 63 +++++++++++++++++++++++++++++++++++---------
net/mptcp/protocol.c | 3 +--
net/mptcp/protocol.h | 3 ++-
net/mptcp/sched.c | 2 +-
4 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 63c5747f0f63..7d0f3421bd04 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,17 @@ 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)
+static struct mptcp_sched_ops *mptcp_pernet_sched(struct mptcp_pernet *pernet)
{
- return mptcp_get_pernet(net)->scheduler;
+ struct mptcp_sched_ops *sched;
+
+ sched = rcu_dereference(pernet->scheduler);
+ return sched ? sched : &mptcp_sched_default;
+}
+
+struct mptcp_sched_ops *mptcp_get_scheduler(const struct net *net)
+{
+ return mptcp_pernet_sched(mptcp_get_pernet(net));
}
unsigned int mptcp_add_addr_v6_port_drop_ts(const struct net *net)
@@ -112,23 +120,33 @@ 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));
+
+ if (bpf_try_module_get(&mptcp_sched_default, mptcp_sched_default.owner))
+ 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;
+ struct mptcp_sched_ops *sched, *prev;
int ret = 0;
rcu_read_lock();
sched = mptcp_sched_find(name);
- if (sched)
- strscpy(scheduler, name, MPTCP_SCHED_NAME_MAX);
- else
+ if (sched) {
+ if (bpf_try_module_get(sched, sched->owner)) {
+ prev = xchg(&pernet->scheduler, sched);
+ if (prev)
+ bpf_module_put(prev, prev->owner);
+ } else {
+ ret = -EBUSY;
+ }
+ } else {
ret = -ENOENT;
+ }
rcu_read_unlock();
return ret;
@@ -137,7 +155,9 @@ 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);
char val[MPTCP_SCHED_NAME_MAX];
struct ctl_table tbl = {
.data = val,
@@ -145,11 +165,13 @@ static int proc_scheduler(const struct ctl_table *ctl, int write,
};
int ret;
- strscpy(val, *scheduler, MPTCP_SCHED_NAME_MAX);
+ rcu_read_lock();
+ strscpy(val, mptcp_pernet_sched(pernet)->name, 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;
}
@@ -563,18 +585,33 @@ void mptcp_active_detect_blackhole(struct sock *ssk, bool expired)
static int __net_init mptcp_net_init(struct net *net)
{
struct mptcp_pernet *pernet = mptcp_get_pernet(net);
+ int ret;
mptcp_pernet_set_defaults(pernet);
- return mptcp_pernet_new_table(net, pernet);
+ ret = mptcp_pernet_new_table(net, pernet);
+ if (ret) {
+ struct mptcp_sched_ops *sched;
+
+ sched = rcu_dereference_protected(pernet->scheduler, true);
+ if (sched)
+ bpf_module_put(sched, sched->owner);
+ }
+
+ return ret;
}
/* Note: the callback will only be called per extra netns */
static void __net_exit mptcp_net_exit(struct net *net)
{
struct mptcp_pernet *pernet = mptcp_get_pernet(net);
+ struct mptcp_sched_ops *sched;
mptcp_pernet_del_table(pernet);
+
+ sched = rcu_dereference_protected(pernet->scheduler, true);
+ if (sched)
+ bpf_module_put(sched, sched->owner);
}
static struct pernet_operations mptcp_pernet_ops = {
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f879b1061f2d..1cbc6b1448bc 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3271,8 +3271,7 @@ 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)));
+ ret = mptcp_init_sched(mptcp_sk(sk), mptcp_get_scheduler(net));
rcu_read_unlock();
if (ret)
return ret;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 7e168e450fb0..6447945be93c 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);
+struct mptcp_sched_ops *mptcp_get_scheduler(const struct net *net);
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] 7+ messages in thread* [PATCH mptcp-next v3 2/5] mptcp: pm: change path_manager sysctl atomically
2026-08-19 12:56 [PATCH mptcp-next v3 0/5] mptcp: avoid data-races around the sysctls Gang Yan
2026-08-19 12:56 ` [PATCH mptcp-next v3 1/5] mptcp: sched: change scheduler sysctl atomically Gang Yan
@ 2026-08-19 12:56 ` Gang Yan
2026-08-19 12:56 ` [PATCH mptcp-next v3 3/5] mptcp: use READ_ONCE() over sysctls Gang Yan
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Gang Yan @ 2026-08-19 12:56 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 (KCSAN), 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.
No module reference is taken on the path manager ops for now, as they
can only be registered from built-in code on one side, and on the other
side the reference counting will be introduced by the last patch of
this series, together with the BPF path manager support.
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 | 33 +++++++++++++++++++++++----------
net/mptcp/pm.c | 3 ++-
net/mptcp/protocol.h | 3 +--
3 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 7d0f3421bd04..76ff2a41ba38 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,20 @@ 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)
+static struct mptcp_pm_ops *mptcp_pernet_pm(struct mptcp_pernet *pernet)
{
- return mptcp_get_pernet(net)->path_manager;
+ struct mptcp_pm_ops *pm_ops;
+
+ pm_ops = rcu_dereference(pernet->path_manager);
+ return pm_ops ? pm_ops : &mptcp_pm_kernel;
+}
+
+void mptcp_get_path_manager(const struct net *net, char *name)
+{
+ rcu_read_lock();
+ strscpy(name, mptcp_pernet_pm(mptcp_get_pernet(net))->name,
+ MPTCP_PM_NAME_MAX);
+ rcu_read_unlock();
}
static struct mptcp_sched_ops *mptcp_pernet_sched(struct mptcp_pernet *pernet)
@@ -124,7 +135,8 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
if (bpf_try_module_get(&mptcp_sched_default, mptcp_sched_default.owner))
RCU_INIT_POINTER(pernet->scheduler, &mptcp_sched_default);
- strscpy(pernet->path_manager, "kernel", sizeof(pernet->path_manager));
+ RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);
+
pernet->add_addr_v6_port_drop_ts = 1;
}
@@ -210,7 +222,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;
@@ -218,7 +230,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();
@@ -232,7 +244,6 @@ 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;
char pm_name[MPTCP_PM_NAME_MAX];
const struct ctl_table tbl = {
.data = pm_name,
@@ -240,11 +251,13 @@ 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();
+ strscpy(pm_name, mptcp_pernet_pm(pernet)->name, 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;
@@ -276,7 +289,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 6447945be93c..bbcf0ecd72d1 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);
struct mptcp_sched_ops *mptcp_get_scheduler(const struct net *net);
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] 7+ messages in thread* [PATCH mptcp-next v3 3/5] mptcp: use READ_ONCE() over sysctls
2026-08-19 12:56 [PATCH mptcp-next v3 0/5] mptcp: avoid data-races around the sysctls Gang Yan
2026-08-19 12:56 ` [PATCH mptcp-next v3 1/5] mptcp: sched: change scheduler sysctl atomically Gang Yan
2026-08-19 12:56 ` [PATCH mptcp-next v3 2/5] mptcp: pm: change path_manager " Gang Yan
@ 2026-08-19 12:56 ` Gang Yan
2026-08-19 12:56 ` [PATCH mptcp-next v3 4/5] mptcp: pm: use WRITE_ONCE() for the pm_type sysctl Gang Yan
2026-08-19 12:56 ` [PATCH mptcp-next v3 5/5] Squash to "mptcp: pm: init and release mptcp_pm_ops" Gang Yan
4 siblings, 0 replies; 7+ messages in thread
From: Gang Yan @ 2026-08-19 12:56 UTC (permalink / raw)
To: mptcp
From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
To avoid KCSAN issues.
This patch is in theory for -net, and will need to be split in multiple
patches, with different Fixes tags. But I prefer to wait for Eric's
patches, as I noticed he already started to modify mptcp_is_enabled:
https://lore.kernel.org/CANn89iLdwhhwLyO6zRjWMEY3t9g60ZE8ZhOVx33ucg_uRETbmQ@mail.gmail.com
Still, keeping this patch in this series, not to forget about it.
Reported-by: Eric Dumazet <edumazet@google.com>
Closes: https://lore.kernel.org/CANn89iL=os-60kDKqMDdyiXuPF5CG=eejS0vmthwpDGXz_Bp8A@mail.gmail.com
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
net/mptcp/ctrl.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 76ff2a41ba38..5a75f9b76d15 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -50,39 +50,39 @@ static struct mptcp_pernet *mptcp_get_pernet(const struct net *net)
int mptcp_is_enabled(const struct net *net)
{
- return mptcp_get_pernet(net)->mptcp_enabled;
+ return READ_ONCE(mptcp_get_pernet(net)->mptcp_enabled);
}
unsigned int mptcp_get_add_addr_timeout(const struct net *net)
{
- return mptcp_get_pernet(net)->add_addr_timeout;
+ return READ_ONCE(mptcp_get_pernet(net)->add_addr_timeout);
}
int mptcp_is_checksum_enabled(const struct net *net)
{
- return mptcp_get_pernet(net)->checksum_enabled;
+ return READ_ONCE(mptcp_get_pernet(net)->checksum_enabled);
}
int mptcp_allow_join_id0(const struct net *net)
{
- return mptcp_get_pernet(net)->allow_join_initial_addr_port;
+ return READ_ONCE(mptcp_get_pernet(net)->allow_join_initial_addr_port);
}
unsigned int mptcp_stale_loss_cnt(const struct net *net)
{
- return mptcp_get_pernet(net)->stale_loss_cnt;
+ return READ_ONCE(mptcp_get_pernet(net)->stale_loss_cnt);
}
unsigned int mptcp_close_timeout(const struct sock *sk)
{
if (sock_flag(sk, SOCK_DEAD))
return TCP_TIMEWAIT_LEN;
- return mptcp_get_pernet(sock_net(sk))->close_timeout;
+ return READ_ONCE(mptcp_get_pernet(sock_net(sk))->close_timeout);
}
int mptcp_get_pm_type(const struct net *net)
{
- return mptcp_get_pernet(net)->pm_type;
+ return READ_ONCE(mptcp_get_pernet(net)->pm_type);
}
static struct mptcp_pm_ops *mptcp_pernet_pm(struct mptcp_pernet *pernet)
@@ -586,7 +586,7 @@ void mptcp_active_detect_blackhole(struct sock *ssk, bool expired)
net = sock_net(ssk);
timeouts = inet_csk(ssk)->icsk_retransmits;
- to_max = mptcp_get_pernet(net)->syn_retrans_before_tcp_fallback;
+ to_max = READ_ONCE(mptcp_get_pernet(net)->syn_retrans_before_tcp_fallback);
if (timeouts == to_max || (timeouts < to_max && expired)) {
subflow->mpc_drop = 1;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH mptcp-next v3 4/5] mptcp: pm: use WRITE_ONCE() for the pm_type sysctl
2026-08-19 12:56 [PATCH mptcp-next v3 0/5] mptcp: avoid data-races around the sysctls Gang Yan
` (2 preceding siblings ...)
2026-08-19 12:56 ` [PATCH mptcp-next v3 3/5] mptcp: use READ_ONCE() over sysctls Gang Yan
@ 2026-08-19 12:56 ` Gang Yan
2026-08-19 12:56 ` [PATCH mptcp-next v3 5/5] Squash to "mptcp: pm: init and release mptcp_pm_ops" Gang Yan
4 siblings, 0 replies; 7+ messages in thread
From: Gang Yan @ 2026-08-19 12:56 UTC (permalink / raw)
To: mptcp
From: Gang Yan <yangang@kylinos.cn>
Write pernet->pm_type with WRITE_ONCE() in proc_path_manager(), pairing
it with the READ_ONCE() readers introduced earlier in this series.
Note that updating net.mptcp.path_manager swaps the ops pointer first,
then writes the derived pm_type: a socket created in between may see
the new ops with the old pm_type. As the net.mptcp.pm_type knob is
deprecated since v6.15 and will be removed, the race is not fixed on
purpose; document it above the assignment so it does not get reported
again.
Suggested-by: Matthieu Baerts <matttbe@kernel.org>
Assisted-by: Claude:GLM5.2
Co-developed-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
net/mptcp/ctrl.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 5a75f9b76d15..87491b961bf2 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -265,7 +265,13 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
pm_type = MPTCP_PM_TYPE_KERNEL;
else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
pm_type = MPTCP_PM_TYPE_USERSPACE;
- pernet->pm_type = pm_type;
+
+ /* Pre-existing race: two sequential writes, a socket
+ * created in between may see the new ops with the old
+ * pm_type. The knob is deprecated since v6.15 and will
+ * be removed: not fixed on purpose.
+ */
+ WRITE_ONCE(pernet->pm_type, pm_type);
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH mptcp-next v3 5/5] Squash to "mptcp: pm: init and release mptcp_pm_ops"
2026-08-19 12:56 [PATCH mptcp-next v3 0/5] mptcp: avoid data-races around the sysctls Gang Yan
` (3 preceding siblings ...)
2026-08-19 12:56 ` [PATCH mptcp-next v3 4/5] mptcp: pm: use WRITE_ONCE() for the pm_type sysctl Gang Yan
@ 2026-08-19 12:56 ` Gang Yan
2026-08-19 13:12 ` sashiko-bot
4 siblings, 1 reply; 7+ messages in thread
From: Gang Yan @ 2026-08-19 12:56 UTC (permalink / raw)
To: mptcp
From: Gang Yan <yangang@kylinos.cn>
This commit introduces the mptcp_pm_ops lifetime handling on sockets
(mptcp_pm_ops_init/release taking a module reference), and would then
be the first one whose per-net path managers can be unloaded while a
pernet still stores them.
mptcp_pm_ops_init() also takes the ops pointer directly instead of the
name, and mptcp_get_path_manager() returns the ops: the redundant
mptcp_pm_find() list walk from the name is avoided, as done for the
scheduler side earlier in this series.
Assisted-by: Claude:GLM5.2
Co-developed-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
net/mptcp/ctrl.c | 35 +++++++++++++++++++++++++----------
net/mptcp/pm.c | 12 ++++--------
net/mptcp/protocol.h | 2 +-
3 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 87491b961bf2..6379a9f481ac 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -93,12 +93,9 @@ static struct mptcp_pm_ops *mptcp_pernet_pm(struct mptcp_pernet *pernet)
return pm_ops ? pm_ops : &mptcp_pm_kernel;
}
-void mptcp_get_path_manager(const struct net *net, char *name)
+struct mptcp_pm_ops *mptcp_get_path_manager(const struct net *net)
{
- rcu_read_lock();
- strscpy(name, mptcp_pernet_pm(mptcp_get_pernet(net))->name,
- MPTCP_PM_NAME_MAX);
- rcu_read_unlock();
+ return mptcp_pernet_pm(mptcp_get_pernet(net));
}
static struct mptcp_sched_ops *mptcp_pernet_sched(struct mptcp_pernet *pernet)
@@ -135,7 +132,8 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
if (bpf_try_module_get(&mptcp_sched_default, mptcp_sched_default.owner))
RCU_INIT_POINTER(pernet->scheduler, &mptcp_sched_default);
- RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);
+ if (bpf_try_module_get(&mptcp_pm_kernel, mptcp_pm_kernel.owner))
+ RCU_INIT_POINTER(pernet->path_manager, &mptcp_pm_kernel);
pernet->add_addr_v6_port_drop_ts = 1;
}
@@ -224,15 +222,22 @@ static int proc_blackhole_detect_timeout(const struct ctl_table *table,
static int mptcp_set_path_manager(struct mptcp_pernet *pernet, const char *name)
{
- struct mptcp_pm_ops *pm_ops;
+ struct mptcp_pm_ops *pm_ops, *prev;
int ret = 0;
rcu_read_lock();
pm_ops = mptcp_pm_find(name);
- if (pm_ops)
- xchg(&pernet->path_manager, pm_ops);
- else
+ if (pm_ops) {
+ if (bpf_try_module_get(pm_ops, pm_ops->owner)) {
+ prev = xchg(&pernet->path_manager, pm_ops);
+ if (prev)
+ bpf_module_put(prev, prev->owner);
+ } else {
+ ret = -EBUSY;
+ }
+ } else {
ret = -ENOENT;
+ }
rcu_read_unlock();
return ret;
@@ -611,10 +616,15 @@ static int __net_init mptcp_net_init(struct net *net)
ret = mptcp_pernet_new_table(net, pernet);
if (ret) {
struct mptcp_sched_ops *sched;
+ struct mptcp_pm_ops *pm;
sched = rcu_dereference_protected(pernet->scheduler, true);
if (sched)
bpf_module_put(sched, sched->owner);
+
+ pm = rcu_dereference_protected(pernet->path_manager, true);
+ if (pm)
+ bpf_module_put(pm, pm->owner);
}
return ret;
@@ -625,12 +635,17 @@ static void __net_exit mptcp_net_exit(struct net *net)
{
struct mptcp_pernet *pernet = mptcp_get_pernet(net);
struct mptcp_sched_ops *sched;
+ struct mptcp_pm_ops *pm;
mptcp_pernet_del_table(pernet);
sched = rcu_dereference_protected(pernet->scheduler, true);
if (sched)
bpf_module_put(sched, sched->owner);
+
+ pm = rcu_dereference_protected(pernet->path_manager, true);
+ if (pm)
+ bpf_module_put(pm, pm->owner);
}
static struct pernet_operations mptcp_pernet_ops = {
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 09f99bcd827c..76158f0ac5a0 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -1155,13 +1155,11 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
spin_unlock_bh(&msk->pm.lock);
}
-static void mptcp_pm_ops_init(struct mptcp_sock *msk, const char *pm_name)
+static void mptcp_pm_ops_init(struct mptcp_sock *msk,
+ struct mptcp_pm_ops *pm_ops)
{
- struct mptcp_pm_ops *pm_ops;
-
- pm_ops = mptcp_pm_find(pm_name);
if (!pm_ops || !bpf_try_module_get(pm_ops, pm_ops->owner)) {
- pr_warn_once("pm %s fails, fallback to default pm", pm_name);
+ pr_warn_once("pm %s fails, fallback to default pm", pm_ops->name);
pm_ops = &mptcp_pm_kernel;
}
@@ -1204,7 +1202,6 @@ 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);
- char pm_name[MPTCP_PM_NAME_MAX];
u8 pm_type = mptcp_get_pm_type(net);
struct mptcp_pm_data *pm = &msk->pm;
@@ -1213,9 +1210,8 @@ 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);
+ mptcp_pm_ops_init(msk, mptcp_get_path_manager(net));
rcu_read_unlock();
}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index bbcf0ecd72d1..1deefb8d3fcc 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);
-void mptcp_get_path_manager(const struct net *net, char *name);
+struct mptcp_pm_ops *mptcp_get_path_manager(const struct net *net);
struct mptcp_sched_ops *mptcp_get_scheduler(const struct net *net);
unsigned int mptcp_add_addr_v6_port_drop_ts(const struct net *net);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH mptcp-next v3 5/5] Squash to "mptcp: pm: init and release mptcp_pm_ops"
2026-08-19 12:56 ` [PATCH mptcp-next v3 5/5] Squash to "mptcp: pm: init and release mptcp_pm_ops" Gang Yan
@ 2026-08-19 13:12 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-19 13:12 UTC (permalink / raw)
To: Gang Yan; +Cc: mptcp
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] msk->pm.ops is accessed concurrently without the socket lock or RCU protection during MP_JOIN processing, which can race with mptcp_disconnect() leading to a NULL pointer dereference or execution of freed module code.
--
--- Patch [5]: [PATCH mptcp-next v3 5/5] Squash to "mptcp: pm: init and release mptcp_pm_ops" ---
commit f7c7fb9e99182263f4dea26b6331ed438faaf974
Author: Gang Yan <yangang@kylinos.cn>
Squash to "mptcp: pm: init and release mptcp_pm_ops"
This commit introduces the mptcp_pm_ops lifetime handling on sockets
(mptcp_pm_ops_init/release taking a module reference), and would then
be the first one whose per-net path managers can be unloaded while a
pernet still stores them.
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 69a38cb48977a..64244a1a01bca 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -1155,13 +1155,11 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
> spin_unlock_bh(&msk->pm.lock);
> }
>
> -static void mptcp_pm_ops_init(struct mptcp_sock *msk, const char *pm_name)
> +static void mptcp_pm_ops_init(struct mptcp_sock *msk,
> + struct mptcp_pm_ops *pm_ops)
> {
[Severity: Critical]
Since this series introduces mptcp_pm_ops lifetime handling and module
references, is there a race condition with concurrent MP_JOIN requests?
When an incoming MP_JOIN request is processed in softirq context,
subflow_token_join_request() gets the socket via mptcp_token_get_sock(),
which doesn't hold lock_sock() or RCU. It then calls:
net/mptcp/pm.c:mptcp_pm_get_local_id() {
...
return msk->pm.ops->get_local_id(msk, &skc_local);
}
Concurrently, if a local user closes the socket and triggers
mptcp_disconnect(msk), it calls mptcp_pm_ops_release() which clears the ops
and drops the module reference:
net/mptcp/pm.c:mptcp_pm_ops_release() {
...
msk->pm.ops = NULL;
...
bpf_module_put(pm_ops, pm_ops->owner);
}
Could this lead to a NULL pointer dereference or executing freed module code
if subflow_token_join_request() executes concurrently with a socket
disconnect?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819125629.49823-1-gang.yan@linux.dev?part=5
^ permalink raw reply [flat|nested] 7+ messages in thread