[RFC PATCH mptcp-next] mptcp: annotate data-races around sysctl reads

From: Tao Cui <[email protected]>

#626 plans a READ_ONCE-over-sysctls series for the string sysctls; this
is the numeric side. Asking whether to send it on its own or fold it in.

The per-netns MPTCP sysctl values (net.mptcp.enabled, add_addr_timeout,
checksum_enabled, allow_join_initial_addr_port, stale_loss_cnt,
close_timeout, pm_type) are written from the sysctl handlers and read
without locking through the ctrl.c accessors.

Add READ_ONCE() on the readers and WRITE_ONCE() on the pm_type store in
proc_path_manager(), matching what is already done for other mptcp
fields (fully_established, local_id, remote_id, sysctl_tcp_wmem[0]).

The string sysctls (path_manager, scheduler) are not covered: they need
atomic replacement, see the tracker below.

KCSAN reproduces the race on net.mptcp.enabled, and the READ_ONCE makes
it go away:

  BUG: KCSAN: data-race in mptcp_is_enabled / proc_dou8vec_minmax

  write to 0xffff8f93c18553e9 of 1 bytes by task 214 on cpu 1:
   proc_dou8vec_minmax+0x1b1/0x200
   proc_sys_call_handler+0x268/0x350
   vfs_write+0x423/0x710

  read to 0xffff8f93c18553e9 of 1 bytes by task 72 on cpu 0:
   mptcp_is_enabled+0x50/0x60
   mptcp_init_sock+0x2a/0x1c0
   inet_create+0x3f9/0x5a0

  value changed: 0x00 -> 0x01

No functional change.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/626
Signed-off-by: Tao Cui <[email protected]>
---
 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 63c5747f0f63..b0ef6aea4eba 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);
 }

 const char *mptcp_get_path_manager(const struct net *net)
@@ -230,7 +230,7 @@ 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;
+			WRITE_ONCE(pernet->pm_type, pm_type);
 		}
 	}

--
2.43.0