All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kuniyuki Iwashima <kuniyu@amazon.com>
To: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Iurii Zaikin <yzaikin@google.com>
Cc: Kuniyuki Iwashima <kuniyu@amazon.com>,
	Kuniyuki Iwashima <kuni1840@gmail.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: [PATCH v1 net 08/16] sysctl: Add proc_dointvec_jiffies_lockless().
Date: Tue, 5 Jul 2022 22:21:22 -0700	[thread overview]
Message-ID: <20220706052130.16368-9-kuniyu@amazon.com> (raw)
In-Reply-To: <20220706052130.16368-1-kuniyu@amazon.com>

A sysctl variable is accessed concurrently, and there is always a chance of
data-race.  So, all readers and writers need some basic protection to avoid
load/store-tearing.

This patch changes proc_dointvec_jiffies() to use READ_ONCE()/WRITE_ONCE()
internally to fix a data-race on the sysctl side.  For now,
proc_dointvec_jiffies() itself is tolerant to a data-race, but we still
need to add annotations on the other subsystem's side.

In case we miss such fixes, this patch converts proc_dointvec_jiffies() to
a wrapper of proc_dointvec_jiffies_lockless().  When we fix a data-race in
the other subsystem, we can explicitly set it as a handler.

Also, this patch removes proc_dointvec_jiffies()'s document and adds
proc_dointvec_jiffies_lockless()'s one so that no one will use
proc_dointvec_jiffies() anymore.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/linux/sysctl.h |  1 +
 kernel/sysctl.c        | 26 ++++++++++++++++++++------
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index c23b6beef748..8747dbc721f5 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -89,6 +89,7 @@ PROC_HANDLER(proc_douintvec_lockless);
 PROC_HANDLER(proc_dointvec_minmax_lockless);
 PROC_HANDLER(proc_douintvec_minmax_lockless);
 PROC_HANDLER(proc_doulongvec_minmax_lockless);
+PROC_HANDLER(proc_dointvec_jiffies_lockless);
 
 /*
  * Register a set of sysctl names by calling register_sysctl_table
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 931ab58985f2..11a1ce837623 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1221,10 +1221,15 @@ static int do_proc_dointvec_jiffies_conv(bool *negp, unsigned long *lvalp,
 	if (write) {
 		if (*lvalp > INT_MAX / HZ)
 			return 1;
-		*valp = *negp ? -(*lvalp*HZ) : (*lvalp*HZ);
+
+		if (*negp)
+			WRITE_ONCE(*valp, -(*lvalp * HZ));
+		else
+			WRITE_ONCE(*valp, *lvalp * HZ);
 	} else {
-		int val = *valp;
+		int val = READ_ONCE(*valp);
 		unsigned long lval;
+
 		if (val < 0) {
 			*negp = true;
 			lval = -(unsigned long)val;
@@ -1286,7 +1291,8 @@ static int do_proc_dointvec_ms_jiffies_conv(bool *negp, unsigned long *lvalp,
 }
 
 /**
- * proc_dointvec_jiffies - read a vector of integers as seconds
+ * proc_dointvec_jiffies_lockless - read/write a vector of integers as
+ * seconds locklessly
  * @table: the sysctl table
  * @write: %TRUE if this is a write to the sysctl file
  * @buffer: the user buffer
@@ -1294,17 +1300,23 @@ static int do_proc_dointvec_ms_jiffies_conv(bool *negp, unsigned long *lvalp,
  * @ppos: file position
  *
  * Reads/writes up to table->maxlen/sizeof(unsigned int) integer
- * values from/to the user buffer, treated as an ASCII string. 
+ * values from/to the user buffer, treated as an ASCII string.
  * The values read are assumed to be in seconds, and are converted into
  * jiffies.
  *
  * Returns 0 on success.
  */
+int proc_dointvec_jiffies_lockless(struct ctl_table *table, int write,
+				   void *buffer, size_t *lenp, loff_t *ppos)
+{
+	return do_proc_dointvec(table, write, buffer, lenp, ppos,
+				do_proc_dointvec_jiffies_conv, NULL);
+}
+
 int proc_dointvec_jiffies(struct ctl_table *table, int write,
 			  void *buffer, size_t *lenp, loff_t *ppos)
 {
-    return do_proc_dointvec(table,write,buffer,lenp,ppos,
-		    	    do_proc_dointvec_jiffies_conv,NULL);
+	return proc_dointvec_jiffies_lockless(table, write, buffer, lenp, ppos);
 }
 
 /**
@@ -1552,6 +1564,7 @@ PROC_HANDLER_ENOSYS(proc_douintvec_lockless);
 PROC_HANDLER_ENOSYS(proc_dointvec_minmax_lockless);
 PROC_HANDLER_ENOSYS(proc_douintvec_minmax_lockless);
 PROC_HANDLER_ENOSYS(proc_doulongvec_minmax_lockless);
+PROC_HANDLER_ENOSYS(proc_dointvec_jiffies_lockless);
 
 #endif /* CONFIG_PROC_SYSCTL */
 
@@ -2468,3 +2481,4 @@ EXPORT_SYMBOL(proc_douintvec_lockless);
 EXPORT_SYMBOL(proc_dointvec_minmax_lockless);
 EXPORT_SYMBOL_GPL(proc_douintvec_minmax_lockless);
 EXPORT_SYMBOL(proc_doulongvec_minmax_lockless);
+EXPORT_SYMBOL(proc_dointvec_jiffies_lockless);
-- 
2.30.2


  parent reply	other threads:[~2022-07-06  5:23 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-06  5:21 [PATCH v1 net 00/16] sysctl: Fix data-races around ipv4_table Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 01/16] sysctl: Clean up proc_handler definitions Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 02/16] sysctl: Add proc_dobool_lockless() Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 03/16] sysctl: Add proc_dointvec_lockless() Kuniyuki Iwashima
2022-07-06  7:00   ` Eric Dumazet
2022-07-06 16:15     ` Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 04/16] sysctl: Add proc_douintvec_lockless() Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 05/16] sysctl: Add proc_dointvec_minmax_lockless() Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 06/16] sysctl: Add proc_douintvec_minmax_lockless() Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 07/16] sysctl: Add proc_doulongvec_minmax_lockless() Kuniyuki Iwashima
2022-07-06  5:21 ` Kuniyuki Iwashima [this message]
2022-07-06  5:21 ` [PATCH v1 net 09/16] tcp: Fix a data-race around sysctl_tcp_max_orphans Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 10/16] inetpeer: Fix data-races around sysctl Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 11/16] net: Fix a data-race around sysctl_mem Kuniyuki Iwashima
2022-07-06 13:17   ` Steven Rostedt
2022-07-06 13:27     ` Steven Rostedt
2022-07-06 16:27       ` Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 12/16] tcp: Mark sysctl_tcp_low_latency obsolete Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 13/16] cipso: Fix a data-race around cipso_v4_cache_bucketsize Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 14/16] cipso: Fix data-races around boolean sysctl Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 15/16] icmp: Fix data-races around sysctl Kuniyuki Iwashima
2022-07-06  5:21 ` [PATCH v1 net 16/16] ipv4: Fix a data-race around sysctl_fib_sync_mem Kuniyuki Iwashima

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220706052130.16368-9-kuniyu@amazon.com \
    --to=kuniyu@amazon.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=keescook@chromium.org \
    --cc=kuba@kernel.org \
    --cc=kuni1840@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=yzaikin@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.