All of lore.kernel.org
 help / color / mirror / Atom feed
* [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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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
  0 siblings, 0 replies; 7+ 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] 7+ messages in thread

end of thread, other threads:[~2026-08-18 12:57 UTC | newest]

Thread overview: 7+ 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

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.