[PATCH v2 1/9] sysctl: support encoding values directly in the table entry

[PATCH v2 1/9] sysctl: support encoding values directly in the table entry

[wenyang.linux]

From: Wen Yang <wenyang.linux@foxmail.com>

Eric points out: "by turning .extra1 and .extra2 into longs instead of
keeping them as pointers and needing constants to be pointed at somewhere
... The only people I can see who find a significant benefit by
consolidating all of the constants into one place are people who know how
to stomp kernel memory."

This patch supports encoding values directly in table entries through the
following work:
- extra1/extra2 and min/max are placed in one union to ensure that the
  previous code is not broken, then we have time to remove unnecessary
  extra1/extra2 progressively;
- since type only has two states, use one bit to represent it;
- two bits were used to represent the information of the above union( 0:
  using extra1/extra2, 1: using min, 2: using max, 3: using both min/max);
- added some helper macros.

Suggested-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Granados <j.granados@samsung.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Iurii Zaikin <yzaikin@google.com>
Cc: linux-kernel@vger.kernel.org
---
 include/linux/sysctl.h | 108 ++++++++++++++++++++++++++++++++++++++---
 kernel/sysctl.c        |  61 +++++++++++++++++------
 2 files changed, 148 insertions(+), 21 deletions(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index ee7d33b89e9e..1ba980219e40 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -61,6 +61,25 @@ extern const int sysctl_vals[];
 
 extern const unsigned long sysctl_long_vals[];
 
+#define	SYSCTL_NUMERIC_NEG_ONE			((long)-1)
+#define	SYSCTL_NUMERIC_ZERO			(0L)
+#define	SYSCTL_NUMERIC_ONE			(1L)
+#define	SYSCTL_NUMERIC_TWO			(2L)
+#define	SYSCTL_NUMERIC_THREE			(3L)
+#define	SYSCTL_NUMERIC_FOUR			(4L)
+#define	SYSCTL_NUMERIC_ONE_HUNDRED		(100L)
+#define	SYSCTL_NUMERIC_TWO_HUNDRED		(200L)
+#define	SYSCTL_NUMERIC_THREE_HUNDRED		(300L)
+#define	SYSCTL_NUMERIC_FIVE_HUNDRED		(500L)
+#define	SYSCTL_NUMERIC_ONE_THOUSAND		(1000L)
+#define	SYSCTL_NUMERIC_TWO_THOUSAND		(2000L)
+#define	SYSCTL_NUMERIC_THREE_THOUSAND		(3000L)
+#define	SYSCTL_NUMERIC_16K			(16384L)
+#define	SYSCTL_NUMERIC_U8_MAX			((long)U8_MAX)
+#define	SYSCTL_NUMERIC_U16_MAX			((long)U16_MAX)
+#define	SYSCTL_NUMERIC_INT_MAX			((long)INT_MAX)
+#define	SYSCTL_NUMERIC_LONG_MAX			(LONG_MAX)
+
 typedef int proc_handler(struct ctl_table *ctl, int write, void *buffer,
 		size_t *lenp, loff_t *ppos);
 
@@ -131,6 +150,18 @@ static inline void *proc_sys_poll_event(struct ctl_table_poll *poll)
 #define DEFINE_CTL_TABLE_POLL(name)					\
 	struct ctl_table_poll name = __CTL_TABLE_POLL_INITIALIZER(name)
 
+enum {
+	SYSCTL_TABLE_TYPE_DEFAULT,
+	SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY
+};
+
+enum {
+	SYSCTL_TABLE_EXTRA_PTR,
+	SYSCTL_TABLE_EXTRA_LONG_INIT_MIN,
+	SYSCTL_TABLE_EXTRA_LONG_INIT_MAX,
+	SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX
+};
+
 /* A sysctl table is an array of struct ctl_table: */
 struct ctl_table {
 	const char *procname;		/* Text ID for /proc/sys, or zero */
@@ -138,20 +169,39 @@ struct ctl_table {
 	int maxlen;
 	umode_t mode;
 	/**
-	 * enum type - Enumeration to differentiate between ctl target types
+	 * type - Indicates to differentiate between ctl target types
 	 * @SYSCTL_TABLE_TYPE_DEFAULT: ctl target with no special considerations
 	 * @SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY: Used to identify a permanently
 	 *                                       empty directory target to serve
 	 *                                       as mount point.
 	 */
-	enum {
-		SYSCTL_TABLE_TYPE_DEFAULT,
-		SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY
-	} type;
+	u8 type:1;
+
+	/**
+	 * extra_flags
+	 * @SYSCTL_TABLE_EXTRA_PTR: flag indicating that this uses extra1/extra2.
+	 * @SYSCTL_TABLE_EXTRA_LONG_INIT_MIN: flag indicating that this uses min/max
+					      and min has been initialized.
+	 * @SYSCTL_TABLE_EXTRA_LONG_INIT_MAX: flag indicating that this uses min/max
+					      and max has been initialized.
+	 * @SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX: flag indicating that this uses min/max
+						 and both have been initialized.
+	 *
+	 */
+	u8 extra_flags:2;
+	union {
+		struct {
+			void *extra1;
+			void *extra2;
+		};
+		struct {
+			long min;
+			long max;
+		};
+	};
+
 	proc_handler *proc_handler;	/* Callback for text formatting */
 	struct ctl_table_poll *poll;
-	void *extra1;
-	void *extra2;
 } __randomize_layout;
 
 struct ctl_node {
@@ -213,6 +263,50 @@ struct ctl_table_root {
 #define register_sysctl(path, table)	\
 	register_sysctl_sz(path, table, ARRAY_SIZE(table))
 
+#define CTL_TABLE_ENTRY(_name, _data, _len, _mode, _func)		      \
+	{								      \
+		.procname = _name,					      \
+		.data = _data,						      \
+		.maxlen = _len,						      \
+		.mode = _mode,						      \
+		.proc_handler = _func,					      \
+		.extra_flags = SYSCTL_TABLE_EXTRA_PTR,			      \
+	}
+
+#define CTL_TABLE_ENTRY_MIN(_name, _data, _len, _mode, _func, _min)	      \
+	{								      \
+		.procname = _name,					      \
+		.data = _data,						      \
+		.maxlen = _len,						      \
+		.mode = _mode,						      \
+		.proc_handler = _func,					      \
+		.min = _min,						      \
+		.extra_flags = SYSCTL_TABLE_EXTRA_LONG_INIT_MIN,	      \
+	}
+
+#define CTL_TABLE_ENTRY_MAX(_name, _data, _len, _mode, _func, _max)	      \
+	{								      \
+		.procname = _name,					      \
+		.data = _data,						      \
+		.maxlen = _len,						      \
+		.mode = _mode,						      \
+		.proc_handler = _func,					      \
+		.max = _max,						      \
+		.extra_flags = SYSCTL_TABLE_EXTRA_LONG_INIT_MAX,	      \
+	}
+
+#define CTL_TABLE_ENTRY_MINMAX(_name, _data, _len, _mode, _func, _min, _max)  \
+	{								      \
+		.procname = _name,					      \
+		.data = _data,						      \
+		.maxlen = _len,						      \
+		.mode = _mode,						      \
+		.proc_handler = _func,					      \
+		.min = _min,						      \
+		.max = _max,						      \
+		.extra_flags = SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX,	      \
+	}
+
 #ifdef CONFIG_SYSCTL
 
 void proc_sys_poll_notify(struct ctl_table_poll *poll);
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 157f7ce2942d..144c441236ab 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -822,6 +822,20 @@ struct do_proc_dointvec_minmax_conv_param {
 	int *max;
 };
 
+static void do_int_conv_param_init(const struct ctl_table *table,
+		struct do_proc_dointvec_minmax_conv_param *param)
+{
+	if (table->extra_flags == SYSCTL_TABLE_EXTRA_PTR) {
+		param->min = table->extra1;
+		param->max = table->extra2;
+	} else {
+		param->min = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MIN) ?
+			(int *)&table->min : NULL;
+		param->max = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MAX) ?
+			(int *)&table->max : NULL;
+	}
+}
+
 static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
 					int *valp,
 					int write, void *data)
@@ -867,10 +881,9 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
 int proc_dointvec_minmax(struct ctl_table *table, int write,
 		  void *buffer, size_t *lenp, loff_t *ppos)
 {
-	struct do_proc_dointvec_minmax_conv_param param = {
-		.min = (int *) table->extra1,
-		.max = (int *) table->extra2,
-	};
+	struct do_proc_dointvec_minmax_conv_param param;
+
+	do_int_conv_param_init(table, &param);
 	return do_proc_dointvec(table, write, buffer, lenp, ppos,
 				do_proc_dointvec_minmax_conv, &param);
 }
@@ -889,6 +902,20 @@ struct do_proc_douintvec_minmax_conv_param {
 	unsigned int *max;
 };
 
+static void do_uint_conv_param_init(const struct ctl_table *table,
+		struct do_proc_douintvec_minmax_conv_param *param)
+{
+	if (table->extra_flags == SYSCTL_TABLE_EXTRA_PTR) {
+		param->min = table->extra1;
+		param->max = table->extra2;
+	} else {
+		param->min = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MIN) ?
+			(unsigned int *)&table->min : NULL;
+		param->max = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MAX) ?
+			(unsigned int *)&table->max : NULL;
+	}
+}
+
 static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
 					 unsigned int *valp,
 					 int write, void *data)
@@ -936,10 +963,9 @@ static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
 int proc_douintvec_minmax(struct ctl_table *table, int write,
 			  void *buffer, size_t *lenp, loff_t *ppos)
 {
-	struct do_proc_douintvec_minmax_conv_param param = {
-		.min = (unsigned int *) table->extra1,
-		.max = (unsigned int *) table->extra2,
-	};
+	struct do_proc_douintvec_minmax_conv_param param;
+
+	do_uint_conv_param_init(table, &param);
 	return do_proc_douintvec(table, write, buffer, lenp, ppos,
 				 do_proc_douintvec_minmax_conv, &param);
 }
@@ -1038,8 +1064,16 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table,
 	}
 
 	i = data;
-	min = table->extra1;
-	max = table->extra2;
+	if (table->extra_flags == SYSCTL_TABLE_EXTRA_PTR) {
+		min = table->extra1;
+		max = table->extra2;
+	} else {
+		min = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MIN) ?
+			&table->min : NULL;
+		max = (table->extra_flags & SYSCTL_TABLE_EXTRA_LONG_INIT_MAX) ?
+			&table->max : NULL;
+	}
+
 	vleft = table->maxlen / sizeof(unsigned long);
 	left = *lenp;
 
@@ -1274,10 +1308,9 @@ int proc_dointvec_jiffies(struct ctl_table *table, int write,
 int proc_dointvec_ms_jiffies_minmax(struct ctl_table *table, int write,
 			  void *buffer, size_t *lenp, loff_t *ppos)
 {
-	struct do_proc_dointvec_minmax_conv_param param = {
-		.min = (int *) table->extra1,
-		.max = (int *) table->extra2,
-	};
+	struct do_proc_dointvec_minmax_conv_param param;
+
+	do_int_conv_param_init(table, &param);
 	return do_proc_dointvec(table, write, buffer, lenp, ppos,
 			do_proc_dointvec_ms_jiffies_minmax_conv, &param);
 }
-- 
2.25.1

[PATCH v2 2/9] kernel/sysctl-test: add some kunit test cases for min/max detection

[wenyang.linux]

From: Wen Yang <wenyang.linux@foxmail.com>

Add some kunit test cases and explicitly check the newly added min/max
initialization behavior. Including basic parsing tests, min/max overflow,
and writing data, etc

Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Granados <j.granados@samsung.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Iurii Zaikin <yzaikin@google.com>
Cc: linux-kernel@vger.kernel.org
---
 kernel/sysctl-test.c | 300 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 300 insertions(+)

diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
index 6ef887c19c48..0a2b19ae2a8c 100644
--- a/kernel/sysctl-test.c
+++ b/kernel/sysctl-test.c
@@ -367,6 +367,300 @@ static void sysctl_test_api_dointvec_write_single_greater_int_max(
 	KUNIT_EXPECT_EQ(test, 0, *((int *)table.data));
 }
 
+/*
+ * Test that writing the int value and check if the min/max are met
+ */
+static void sysctl_test_api_dointvec_write_single_with_minmax_check(
+		struct kunit *test)
+{
+	int data = 0;
+	struct ctl_table table = CTL_TABLE_ENTRY_MINMAX("foo",
+							&data,
+							sizeof(int),
+							0644,
+							proc_dointvec_minmax,
+							SYSCTL_NUMERIC_NEG_ONE,
+							SYSCTL_NUMERIC_ONE_HUNDRED);
+	size_t max_len = 32, len;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	loff_t pos = 0;
+	int i;
+
+	for (i = SYSCTL_NUMERIC_NEG_ONE; i <= SYSCTL_NUMERIC_ONE_HUNDRED; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%d", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, 0,
+				proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, i, data);
+	}
+
+	data = 0;
+	for (i = -10; i < SYSCTL_NUMERIC_NEG_ONE; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%d", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, -EINVAL,
+				proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, 0, data);
+	}
+
+	for (i = SYSCTL_NUMERIC_ONE_HUNDRED + 1; i < 110; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%d", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, -EINVAL,
+				proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, 0, data);
+	}
+}
+
+/*
+ * Test that writing the int value and check if the min is met
+ */
+static void sysctl_test_api_dointvec_write_single_with_min_check(
+		struct kunit *test)
+{
+	size_t max_len = 32, len;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	int data = 0, i;
+	loff_t pos = 0;
+	struct ctl_table table = CTL_TABLE_ENTRY_MIN("bar",
+			&data,
+			sizeof(int),
+			0644,
+			proc_dointvec_minmax,
+			-10);
+
+	for (i = -10; i <= SYSCTL_NUMERIC_ONE_HUNDRED; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%d", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, 0,
+				proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, i, data);
+	}
+
+	data = 0;
+	for (i = -20; i < -10; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%d", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, -EINVAL,
+				proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, 0, data);
+	}
+}
+
+/*
+ * Test that writing the int value and check if the max is met
+ */
+static void sysctl_test_api_dointvec_write_single_with_max_check(
+		struct kunit *test)
+{
+	size_t max_len = 32, len;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	loff_t pos = 0;
+	int data = 0, i;
+	struct ctl_table table = CTL_TABLE_ENTRY_MAX("qux",
+			&data,
+			sizeof(int),
+			0644,
+			proc_dointvec_minmax,
+			SYSCTL_NUMERIC_ONE_HUNDRED);
+
+	for (i = -20; i <= SYSCTL_NUMERIC_ONE_HUNDRED; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%d", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, 0,
+				proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, i, data);
+	}
+
+	data = 0;
+	for (i = SYSCTL_NUMERIC_ONE_HUNDRED + 1; i < SYSCTL_NUMERIC_TWO_HUNDRED; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%d", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, -EINVAL,
+				proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, 0, data);
+	}
+}
+
+/*
+ * Test that writing the unsigned int value and check if the min/max are met
+ */
+static void sysctl_test_api_douintvec_write_single_with_minmax_check(
+		struct kunit *test)
+{
+	unsigned int data = 0;
+	struct ctl_table table1 = CTL_TABLE_ENTRY_MINMAX("foo",
+							 &data,
+							 sizeof(unsigned int),
+							 0644,
+							 proc_douintvec_minmax,
+							 SYSCTL_NUMERIC_ZERO,
+							 SYSCTL_NUMERIC_ONE_THOUSAND);
+	size_t max_len = 32, len;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	loff_t pos = 0;
+	int i;
+
+	for (i = SYSCTL_NUMERIC_ZERO; i <= SYSCTL_NUMERIC_ONE_THOUSAND; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%d", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, 0, proc_douintvec_minmax(&table1, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, i, data);
+	}
+
+	data = 0;
+	for (i = -10; i < 0; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%d", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, -EINVAL, proc_douintvec_minmax(&table1, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, 0, data);
+	}
+
+	for (i = SYSCTL_NUMERIC_ONE_THOUSAND + 1; i < SYSCTL_NUMERIC_ONE_THOUSAND + 10; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%d", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, -EINVAL, proc_douintvec_minmax(&table1, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, 0, data);
+	}
+}
+
+/*
+ * Test that writing the unsigned int value and check if the min is met
+ */
+static void sysctl_test_api_douintvec_write_single_with_min_check(
+		struct kunit *test)
+{
+	size_t max_len = 32, len;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	loff_t pos = 0;
+	unsigned int data = 0, i;
+	struct ctl_table table = CTL_TABLE_ENTRY_MIN("bar",
+			&data,
+			sizeof(unsigned int),
+			0644,
+			proc_douintvec_minmax,
+			SYSCTL_NUMERIC_FOUR);
+
+	for (i = SYSCTL_NUMERIC_FOUR; i <= SYSCTL_NUMERIC_ONE_THOUSAND; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%u", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, 0,
+				proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, i, data);
+	}
+
+	data = 0;
+	for (i = SYSCTL_NUMERIC_ZERO; i < SYSCTL_NUMERIC_FOUR; i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%u", i),
+				max_len);
+		len = strlen(buffer);
+
+		KUNIT_EXPECT_EQ(test, -EINVAL,
+				proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, 0, data);
+	}
+}
+
+/*
+ * Test that writing the unsigned int value and check if the max is met
+ */
+static void sysctl_test_api_douintvec_write_single_with_max_check(
+		struct kunit *test)
+{
+	size_t max_len = 32, len;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	loff_t pos = 0;
+	unsigned int data = 0, i;
+	struct ctl_table table = CTL_TABLE_ENTRY_MAX("bar",
+			&data,
+			sizeof(unsigned int),
+			0644,
+			proc_douintvec_minmax,
+			SYSCTL_NUMERIC_TWO_THOUSAND);
+
+	for (i = SYSCTL_NUMERIC_ONE_THOUSAND; i <= SYSCTL_NUMERIC_TWO_THOUSAND;
+			i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%u", i),
+				max_len);
+		len = strlen(buffer);
+		KUNIT_EXPECT_EQ(test, 0,
+				proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, i, data);
+	}
+
+	data = 0;
+	for (i = SYSCTL_NUMERIC_TWO_THOUSAND + 1;
+			i <  SYSCTL_NUMERIC_THREE_THOUSAND;
+			i++) {
+		pos = 0;
+		KUNIT_ASSERT_LT(test,
+				(size_t)snprintf(buffer, max_len, "%u", i),
+				max_len);
+		len = strlen(buffer);
+
+		KUNIT_EXPECT_EQ(test, -EINVAL,
+				proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+					user_buffer, &len, &pos));
+		KUNIT_EXPECT_EQ(test, 0, data);
+	}
+}
+
 static struct kunit_case sysctl_test_cases[] = {
 	KUNIT_CASE(sysctl_test_api_dointvec_null_tbl_data),
 	KUNIT_CASE(sysctl_test_api_dointvec_table_maxlen_unset),
@@ -378,6 +672,12 @@ static struct kunit_case sysctl_test_cases[] = {
 	KUNIT_CASE(sysctl_test_dointvec_write_happy_single_negative),
 	KUNIT_CASE(sysctl_test_api_dointvec_write_single_less_int_min),
 	KUNIT_CASE(sysctl_test_api_dointvec_write_single_greater_int_max),
+	KUNIT_CASE(sysctl_test_api_dointvec_write_single_with_minmax_check),
+	KUNIT_CASE(sysctl_test_api_dointvec_write_single_with_min_check),
+	KUNIT_CASE(sysctl_test_api_dointvec_write_single_with_max_check),
+	KUNIT_CASE(sysctl_test_api_douintvec_write_single_with_minmax_check),
+	KUNIT_CASE(sysctl_test_api_douintvec_write_single_with_min_check),
+	KUNIT_CASE(sysctl_test_api_douintvec_write_single_with_max_check),
 	{}
 };
 
-- 
2.25.1

[PATCH v2 3/9] rxrpc: delete these unnecessary static variables n_65535, four, max_500, etc

[wenyang.linux]

From: Wen Yang <wenyang.linux@foxmail.com>

Delete unnecessary static variables(n_65535, four, max_500, etc.)
and encode them directly in the table entry.

Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Granados <j.granados@samsung.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Marc Dionne <marc.dionne@auristor.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 net/rxrpc/sysctl.c | 169 +++++++++++++++++++--------------------------
 1 file changed, 70 insertions(+), 99 deletions(-)

diff --git a/net/rxrpc/sysctl.c b/net/rxrpc/sysctl.c
index ecaeb4ecfb58..26ec6eff8402 100644
--- a/net/rxrpc/sysctl.c
+++ b/net/rxrpc/sysctl.c
@@ -11,15 +11,6 @@
 #include "ar-internal.h"
 
 static struct ctl_table_header *rxrpc_sysctl_reg_table;
-static const unsigned int four = 4;
-static const unsigned int max_backlog = RXRPC_BACKLOG_MAX - 1;
-static const unsigned int n_65535 = 65535;
-static const unsigned int n_max_acks = 255;
-static const unsigned long one_jiffy = 1;
-static const unsigned long max_jiffies = MAX_JIFFY_OFFSET;
-#ifdef CONFIG_AF_RXRPC_INJECT_RX_DELAY
-static const unsigned long max_500 = 500;
-#endif
 
 /*
  * RxRPC operating parameters.
@@ -29,102 +20,82 @@ static const unsigned long max_500 = 500;
  */
 static struct ctl_table rxrpc_sysctl_table[] = {
 	/* Values measured in milliseconds but used in jiffies */
-	{
-		.procname	= "soft_ack_delay",
-		.data		= &rxrpc_soft_ack_delay,
-		.maxlen		= sizeof(unsigned long),
-		.mode		= 0644,
-		.proc_handler	= proc_doulongvec_ms_jiffies_minmax,
-		.extra1		= (void *)&one_jiffy,
-		.extra2		= (void *)&max_jiffies,
-	},
-	{
-		.procname	= "idle_ack_delay",
-		.data		= &rxrpc_idle_ack_delay,
-		.maxlen		= sizeof(unsigned long),
-		.mode		= 0644,
-		.proc_handler	= proc_doulongvec_ms_jiffies_minmax,
-		.extra1		= (void *)&one_jiffy,
-		.extra2		= (void *)&max_jiffies,
-	},
-	{
-		.procname	= "idle_conn_expiry",
-		.data		= &rxrpc_conn_idle_client_expiry,
-		.maxlen		= sizeof(unsigned long),
-		.mode		= 0644,
-		.proc_handler	= proc_doulongvec_ms_jiffies_minmax,
-		.extra1		= (void *)&one_jiffy,
-		.extra2		= (void *)&max_jiffies,
-	},
-	{
-		.procname	= "idle_conn_fast_expiry",
-		.data		= &rxrpc_conn_idle_client_fast_expiry,
-		.maxlen		= sizeof(unsigned long),
-		.mode		= 0644,
-		.proc_handler	= proc_doulongvec_ms_jiffies_minmax,
-		.extra1		= (void *)&one_jiffy,
-		.extra2		= (void *)&max_jiffies,
-	},
+	CTL_TABLE_ENTRY_MINMAX("soft_ack_delay",
+			       &rxrpc_soft_ack_delay,
+			       sizeof(unsigned long),
+			       0644,
+			       proc_doulongvec_ms_jiffies_minmax,
+			       SYSCTL_NUMERIC_ONE,
+			       MAX_JIFFY_OFFSET),
+	CTL_TABLE_ENTRY_MINMAX("idle_ack_delay",
+			       &rxrpc_idle_ack_delay,
+			       sizeof(unsigned long),
+			       0644,
+			       proc_doulongvec_ms_jiffies_minmax,
+			       SYSCTL_NUMERIC_ONE,
+			       MAX_JIFFY_OFFSET),
+	CTL_TABLE_ENTRY_MINMAX("idle_conn_expiry",
+			       &rxrpc_conn_idle_client_expiry,
+			       sizeof(unsigned long),
+			       0644,
+			       proc_doulongvec_ms_jiffies_minmax,
+			       SYSCTL_NUMERIC_ONE,
+			       MAX_JIFFY_OFFSET),
+	CTL_TABLE_ENTRY_MINMAX("idle_conn_fast_expiry",
+			       &rxrpc_conn_idle_client_fast_expiry,
+			       sizeof(unsigned long),
+			       0644,
+			       proc_doulongvec_ms_jiffies_minmax,
+			       SYSCTL_NUMERIC_ONE,
+			       MAX_JIFFY_OFFSET),
 
 	/* Values used in milliseconds */
 #ifdef CONFIG_AF_RXRPC_INJECT_RX_DELAY
-	{
-		.procname	= "inject_rx_delay",
-		.data		= &rxrpc_inject_rx_delay,
-		.maxlen		= sizeof(unsigned long),
-		.mode		= 0644,
-		.proc_handler	= proc_doulongvec_minmax,
-		.extra1		= (void *)SYSCTL_LONG_ZERO,
-		.extra2		= (void *)&max_500,
-	},
+	CTL_TABLE_ENTRY_MINMAX("inject_rx_delay",
+			       &rxrpc_inject_rx_delay,
+			       sizeof(unsigned long),
+			       0644,
+			       proc_doulongvec_minmax,
+			       SYSCTL_NUMERIC_ZERO,
+			       SYSCTL_NUMERIC_FIVE_HUNDRED),
 #endif
 
 	/* Non-time values */
-	{
-		.procname	= "reap_client_conns",
-		.data		= &rxrpc_reap_client_connections,
-		.maxlen		= sizeof(unsigned int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= (void *)SYSCTL_ONE,
-		.extra2		= (void *)&n_65535,
-	},
-	{
-		.procname	= "max_backlog",
-		.data		= &rxrpc_max_backlog,
-		.maxlen		= sizeof(unsigned int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= (void *)&four,
-		.extra2		= (void *)&max_backlog,
-	},
-	{
-		.procname	= "rx_window_size",
-		.data		= &rxrpc_rx_window_size,
-		.maxlen		= sizeof(unsigned int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= (void *)SYSCTL_ONE,
-		.extra2		= (void *)&n_max_acks,
-	},
-	{
-		.procname	= "rx_mtu",
-		.data		= &rxrpc_rx_mtu,
-		.maxlen		= sizeof(unsigned int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= (void *)SYSCTL_ONE,
-		.extra2		= (void *)&n_65535,
-	},
-	{
-		.procname	= "rx_jumbo_max",
-		.data		= &rxrpc_rx_jumbo_max,
-		.maxlen		= sizeof(unsigned int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= (void *)SYSCTL_ONE,
-		.extra2		= (void *)&four,
-	},
+	CTL_TABLE_ENTRY_MINMAX("reap_client_conns",
+			       &rxrpc_reap_client_connections,
+			       sizeof(unsigned int),
+			       0644,
+			       proc_dointvec_minmax,
+			       SYSCTL_NUMERIC_ONE,
+			       SYSCTL_NUMERIC_U16_MAX),
+	CTL_TABLE_ENTRY_MINMAX("max_backlog",
+			       &rxrpc_max_backlog,
+			       sizeof(unsigned int),
+			       0644,
+			       proc_dointvec_minmax,
+			       SYSCTL_NUMERIC_FOUR,
+			       RXRPC_BACKLOG_MAX - 1),
+	CTL_TABLE_ENTRY_MINMAX("rx_window_size",
+			       &rxrpc_rx_window_size,
+			       sizeof(unsigned int),
+			       0644,
+			       proc_dointvec_minmax,
+			       SYSCTL_NUMERIC_ONE,
+			       SYSCTL_NUMERIC_U8_MAX),
+	CTL_TABLE_ENTRY_MINMAX("rx_mtu",
+			       &rxrpc_rx_mtu,
+			       sizeof(unsigned int),
+			       0644,
+			       proc_dointvec_minmax,
+			       SYSCTL_NUMERIC_ONE,
+			       SYSCTL_NUMERIC_U16_MAX),
+	CTL_TABLE_ENTRY_MINMAX("rx_jumbo_max",
+			       &rxrpc_rx_jumbo_max,
+			       sizeof(unsigned int),
+			       0644,
+			       proc_dointvec_minmax,
+			       SYSCTL_NUMERIC_ONE,
+			       SYSCTL_NUMERIC_FOUR),
 	{ }
 };
 
-- 
2.25.1

[PATCH v2 4/9] net: ipv6: delete these unnecessary static variables two_five_five and minus_one

[wenyang.linux]

From: Wen Yang <wenyang.linux@foxmail.com>

Delete unnecessary static variables (two_five_five and minux_one)
and encode them directly in the table entry.

Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Granados <j.granados@samsung.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: David Ahern <dsahern@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 net/ipv6/addrconf.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 733ace18806c..73776ff76365 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6660,8 +6660,6 @@ static int addrconf_sysctl_disable_policy(struct ctl_table *ctl, int write,
 	return ret;
 }
 
-static int minus_one = -1;
-static const int two_five_five = 255;
 static u32 ioam6_if_id_max = U16_MAX;
 
 static const struct ctl_table addrconf_sysctl[] = {
@@ -6678,8 +6676,9 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= (void *)SYSCTL_ONE,
-		.extra2		= (void *)&two_five_five,
+		.min		= SYSCTL_NUMERIC_ONE,
+		.max		= SYSCTL_NUMERIC_U8_MAX,
+		.extra_flags	= SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX,
 	},
 	{
 		.procname	= "mtu",
@@ -6722,7 +6721,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &minus_one,
+		.min		= SYSCTL_NUMERIC_NEG_ONE,
+		.extra_flags	= SYSCTL_TABLE_EXTRA_LONG_INIT_MIN,
 	},
 	{
 		.procname	= "router_solicitation_interval",
@@ -7061,8 +7061,9 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= (void *)SYSCTL_ZERO,
-		.extra2		= (void *)&two_five_five,
+		.min		= SYSCTL_NUMERIC_ZERO,
+		.max		= SYSCTL_NUMERIC_U8_MAX,
+		.extra_flags	= SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX,
 	},
 	{
 		.procname	= "rpl_seg_enabled",
-- 
2.25.1

[PATCH v2 5/9] svcrdma: delete these unnecessary static variables min_ord, max_ord, etc

[wenyang.linux]

From: Wen Yang <wenyang.linux@foxmail.com>

Delete unnecessary static variables (min_ord, max_ord, etc.)
and encode them directly in the table entry.

Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Granados <j.granados@samsung.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: David Ahern <dsahern@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 net/sunrpc/xprtrdma/svc_rdma.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/net/sunrpc/xprtrdma/svc_rdma.c b/net/sunrpc/xprtrdma/svc_rdma.c
index f86970733eb0..98cde9c2bf5a 100644
--- a/net/sunrpc/xprtrdma/svc_rdma.c
+++ b/net/sunrpc/xprtrdma/svc_rdma.c
@@ -53,15 +53,9 @@
 
 /* RPC/RDMA parameters */
 unsigned int svcrdma_ord = 16;	/* historical default */
-static unsigned int min_ord = 1;
-static unsigned int max_ord = 255;
 unsigned int svcrdma_max_requests = RPCRDMA_MAX_REQUESTS;
 unsigned int svcrdma_max_bc_requests = RPCRDMA_MAX_BC_REQUESTS;
-static unsigned int min_max_requests = 4;
-static unsigned int max_max_requests = 16384;
 unsigned int svcrdma_max_req_size = RPCRDMA_DEF_INLINE_THRESH;
-static unsigned int min_max_inline = RPCRDMA_DEF_INLINE_THRESH;
-static unsigned int max_max_inline = RPCRDMA_MAX_INLINE_THRESH;
 static unsigned int svcrdma_stat_unused;
 static unsigned int zero;
 
@@ -114,8 +108,9 @@ static struct ctl_table svcrdma_parm_table[] = {
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &min_max_requests,
-		.extra2		= &max_max_requests
+		.min		= SYSCTL_NUMERIC_FOUR,
+		.max		= SYSCTL_NUMERIC_16K,
+		.extra_flags	= SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX,
 	},
 	{
 		.procname	= "max_req_size",
@@ -123,8 +118,9 @@ static struct ctl_table svcrdma_parm_table[] = {
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &min_max_inline,
-		.extra2		= &max_max_inline
+		.min		= RPCRDMA_DEF_INLINE_THRESH,
+		.max		= RPCRDMA_MAX_INLINE_THRESH,
+		.extra_flags	= SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX,
 	},
 	{
 		.procname	= "max_outbound_read_requests",
@@ -132,8 +128,9 @@ static struct ctl_table svcrdma_parm_table[] = {
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &min_ord,
-		.extra2		= &max_ord,
+		.min		= SYSCTL_NUMERIC_ONE,
+		.max		= SYSCTL_NUMERIC_U8_MAX,
+		.extra_flags	= SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX,
 	},
 
 	{
-- 
2.25.1

[PATCH v2 6/9] sysctl: delete these unnecessary static variables i_zero and i_one_hundred

[wenyang.linux]

From: Wen Yang <wenyang.linux@foxmail.com>

Delete unnecessary static variables (i_zero and i_one_hundred)
and encode them directly in the table entry.

Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Granados <j.granados@samsung.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: linux-kernel@vger.kernel.org
---
 lib/test_sysctl.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c
index 9321d850931f..cf298f25d686 100644
--- a/lib/test_sysctl.c
+++ b/lib/test_sysctl.c
@@ -26,8 +26,6 @@
 #include <linux/delay.h>
 #include <linux/vmalloc.h>
 
-static int i_zero;
-static int i_one_hundred = 100;
 static int match_int_ok = 1;
 
 
@@ -78,8 +76,9 @@ static struct ctl_table test_table[] = {
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= &i_zero,
-		.extra2         = &i_one_hundred,
+		.min		= SYSCTL_NUMERIC_ZERO,
+		.max		= SYSCTL_NUMERIC_ONE_HUNDRED,
+		.extra_flags	= SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX,
 	},
 	{
 		.procname	= "int_0002",
@@ -108,8 +107,9 @@ static struct ctl_table test_table[] = {
 		.maxlen		= sizeof(test_data.boot_int),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
-		.extra1		= SYSCTL_ZERO,
-		.extra2         = SYSCTL_ONE,
+		.min		= SYSCTL_NUMERIC_ZERO,
+		.max		= SYSCTL_NUMERIC_ONE,
+		.extra_flags	= SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX,
 	},
 	{
 		.procname	= "uint_0001",
-- 
2.25.1

[PATCH v2 7/9] epoll: delete these unnecessary static variables long_zero and long_max

[wenyang.linux]

From: Wen Yang <wenyang.linux@foxmail.com>

Delete unnecessary static variables (long_zero and long_max)
and encode them directly in the table entry.

Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Granados <j.granados@samsung.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: "Darrick J. Wong" <djwong@kernel.org>
Cc: linux-kernel@vger.kernel.org
---
 fs/eventpoll.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 3534d36a1474..7cfc4fb0ca3c 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -309,19 +309,14 @@ static void unlist_file(struct epitems_head *head)
 
 #include <linux/sysctl.h>
 
-static long long_zero;
-static long long_max = LONG_MAX;
-
 static struct ctl_table epoll_table[] = {
-	{
-		.procname	= "max_user_watches",
-		.data		= &max_user_watches,
-		.maxlen		= sizeof(max_user_watches),
-		.mode		= 0644,
-		.proc_handler	= proc_doulongvec_minmax,
-		.extra1		= &long_zero,
-		.extra2		= &long_max,
-	},
+	CTL_TABLE_ENTRY_MINMAX("max_user_watches",
+				&max_user_watches,
+				sizeof(max_user_watches),
+				0644,
+				proc_doulongvec_minmax,
+				SYSCTL_NUMERIC_ZERO,
+				SYSCTL_NUMERIC_LONG_MAX),
 };
 
 static void __init epoll_sysctls_init(void)
-- 
2.25.1

[PATCH v2 8/9] fs: inotify: delete these unnecessary static variables it_zero and it_int_max

[wenyang.linux]

From: Wen Yang <wenyang.linux@foxmail.com>

Delete unnecessary static variables (it_zero and it_int_max)
and encode them directly in the table entry.

Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Granados <j.granados@samsung.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: "Darrick J. Wong" <djwong@kernel.org>
Cc: linux-kernel@vger.kernel.org
---
 fs/notify/inotify/inotify_user.c | 49 +++++++++++++-------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 85d8fdd55329..b346d61179ea 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -55,36 +55,27 @@ struct kmem_cache *inotify_inode_mark_cachep __ro_after_init;
 
 #include <linux/sysctl.h>
 
-static long it_zero = 0;
-static long it_int_max = INT_MAX;
-
 static struct ctl_table inotify_table[] = {
-	{
-		.procname	= "max_user_instances",
-		.data		= &init_user_ns.ucount_max[UCOUNT_INOTIFY_INSTANCES],
-		.maxlen		= sizeof(long),
-		.mode		= 0644,
-		.proc_handler	= proc_doulongvec_minmax,
-		.extra1		= &it_zero,
-		.extra2		= &it_int_max,
-	},
-	{
-		.procname	= "max_user_watches",
-		.data		= &init_user_ns.ucount_max[UCOUNT_INOTIFY_WATCHES],
-		.maxlen		= sizeof(long),
-		.mode		= 0644,
-		.proc_handler	= proc_doulongvec_minmax,
-		.extra1		= &it_zero,
-		.extra2		= &it_int_max,
-	},
-	{
-		.procname	= "max_queued_events",
-		.data		= &inotify_max_queued_events,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= SYSCTL_ZERO
-	},
+	CTL_TABLE_ENTRY_MINMAX("max_user_instances",
+			       &init_user_ns.ucount_max[UCOUNT_INOTIFY_INSTANCES],
+			       sizeof(long),
+			       0644,
+			       proc_doulongvec_minmax,
+			       SYSCTL_NUMERIC_ZERO,
+			       SYSCTL_NUMERIC_INT_MAX),
+	CTL_TABLE_ENTRY_MINMAX("max_user_watches",
+			       &init_user_ns.ucount_max[UCOUNT_INOTIFY_WATCHES],
+			       sizeof(long),
+			       0644,
+			       proc_doulongvec_minmax,
+			       SYSCTL_NUMERIC_ZERO,
+			       SYSCTL_NUMERIC_INT_MAX),
+	CTL_TABLE_ENTRY_MIN("max_queued_events",
+			    &inotify_max_queued_events,
+			    sizeof(int),
+			    0644,
+			    proc_dointvec_minmax,
+			    SYSCTL_NUMERIC_ZERO),
 };
 
 static void __init inotify_sysctls_init(void)
-- 
2.25.1

[PATCH v2 9/9] ucounts: delete these unnecessary static variables ue_zero and ue_int_max

[wenyang.linux]

From: Wen Yang <wenyang.linux@foxmail.com>

Delete unnecessary static variables (ue_zero and ue_int_max)
and encode them directly in the table entry.

Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Granados <j.granados@samsung.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org
---
 kernel/ucount.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/kernel/ucount.c b/kernel/ucount.c
index e188c25ed2b3..53d96cb27309 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -58,17 +58,15 @@ static struct ctl_table_root set_root = {
 	.permissions = set_permissions,
 };
 
-static long ue_zero = 0;
-static long ue_int_max = INT_MAX;
-
 #define UCOUNT_ENTRY(name)					\
 	{							\
 		.procname	= name,				\
 		.maxlen		= sizeof(long),			\
 		.mode		= 0644,				\
 		.proc_handler	= proc_doulongvec_minmax,	\
-		.extra1		= &ue_zero,			\
-		.extra2		= &ue_int_max,			\
+		.min		= SYSCTL_NUMERIC_ZERO,		\
+		.max		= SYSCTL_NUMERIC_INT_MAX,	\
+		.extra_flags	= SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX \
 	}
 static struct ctl_table user_table[] = {
 	UCOUNT_ENTRY("max_user_namespaces"),
-- 
2.25.1

Re: [PATCH v2 1/9] sysctl: support encoding values directly in the table entry

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 6441 bytes --]

On Sat, Mar 09, 2024 at 06:31:18PM +0800, wenyang.linux@foxmail.com wrote:
> From: Wen Yang <wenyang.linux@foxmail.com>
> 
> Eric points out: "by turning .extra1 and .extra2 into longs instead of
> keeping them as pointers and needing constants to be pointed at somewhere
> ... The only people I can see who find a significant benefit by
> consolidating all of the constants into one place are people who know how
> to stomp kernel memory."
I'm assuming that this is the "why" of the commit. Please change it so
it is more direct. Something like "Directly encode numeric values in
macros in order to ...". If you want to add Eric's opinion please add it
as a link (Please follow Documentation/process/submitting-patches.rst)


> 
> This patch supports encoding values directly in table entries through the
> following work:
> - extra1/extra2 and min/max are placed in one union to ensure that the
>   previous code is not broken, then we have time to remove unnecessary
>   extra1/extra2 progressively;
> - since type only has two states, use one bit to represent it;
Please remove this optimization from your commit. This will conflict
with work that Thomas is doing here
https://lore.kernel.org/all/20240222-sysctl-empty-dir-v1-0-45ba9a6352e8@weissschuh.net 

> - two bits were used to represent the information of the above union( 0:
>   using extra1/extra2, 1: using min, 2: using max, 3: using both min/max);
> - added some helper macros.
> 
> Suggested-by: Eric W. Biederman <ebiederm@xmission.com>
> Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Joel Granados <j.granados@samsung.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Iurii Zaikin <yzaikin@google.com>
> Cc: linux-kernel@vger.kernel.org
> ---
>  include/linux/sysctl.h | 108 ++++++++++++++++++++++++++++++++++++++---
>  kernel/sysctl.c        |  61 +++++++++++++++++------
>  2 files changed, 148 insertions(+), 21 deletions(-)
> 
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index ee7d33b89e9e..1ba980219e40 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -61,6 +61,25 @@ extern const int sysctl_vals[];
>  
>  extern const unsigned long sysctl_long_vals[];
>  
> +#define	SYSCTL_NUMERIC_NEG_ONE			((long)-1)
> +#define	SYSCTL_NUMERIC_ZERO			(0L)
> +#define	SYSCTL_NUMERIC_ONE			(1L)
> +#define	SYSCTL_NUMERIC_TWO			(2L)
> +#define	SYSCTL_NUMERIC_THREE			(3L)
> +#define	SYSCTL_NUMERIC_FOUR			(4L)
> +#define	SYSCTL_NUMERIC_ONE_HUNDRED		(100L)
> +#define	SYSCTL_NUMERIC_TWO_HUNDRED		(200L)
> +#define	SYSCTL_NUMERIC_THREE_HUNDRED		(300L)
> +#define	SYSCTL_NUMERIC_FIVE_HUNDRED		(500L)
> +#define	SYSCTL_NUMERIC_ONE_THOUSAND		(1000L)
> +#define	SYSCTL_NUMERIC_TWO_THOUSAND		(2000L)
> +#define	SYSCTL_NUMERIC_THREE_THOUSAND		(3000L)
> +#define	SYSCTL_NUMERIC_16K			(16384L)
> +#define	SYSCTL_NUMERIC_U8_MAX			((long)U8_MAX)
> +#define	SYSCTL_NUMERIC_U16_MAX			((long)U16_MAX)
> +#define	SYSCTL_NUMERIC_INT_MAX			((long)INT_MAX)
> +#define	SYSCTL_NUMERIC_LONG_MAX			(LONG_MAX)
> +
>  typedef int proc_handler(struct ctl_table *ctl, int write, void *buffer,
>  		size_t *lenp, loff_t *ppos);
>  
> @@ -131,6 +150,18 @@ static inline void *proc_sys_poll_event(struct ctl_table_poll *poll)
>  #define DEFINE_CTL_TABLE_POLL(name)					\
>  	struct ctl_table_poll name = __CTL_TABLE_POLL_INITIALIZER(name)
>  
> +enum {
> +	SYSCTL_TABLE_TYPE_DEFAULT,
> +	SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY
> +};
> +
> +enum {
> +	SYSCTL_TABLE_EXTRA_PTR,
> +	SYSCTL_TABLE_EXTRA_LONG_INIT_MIN,
> +	SYSCTL_TABLE_EXTRA_LONG_INIT_MAX,
> +	SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX
> +};
> +
>  /* A sysctl table is an array of struct ctl_table: */
>  struct ctl_table {
>  	const char *procname;		/* Text ID for /proc/sys, or zero */
> @@ -138,20 +169,39 @@ struct ctl_table {
>  	int maxlen;
>  	umode_t mode;
>  	/**
> -	 * enum type - Enumeration to differentiate between ctl target types
> +	 * type - Indicates to differentiate between ctl target types
>  	 * @SYSCTL_TABLE_TYPE_DEFAULT: ctl target with no special considerations
>  	 * @SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY: Used to identify a permanently
>  	 *                                       empty directory target to serve
>  	 *                                       as mount point.
>  	 */
> -	enum {
> -		SYSCTL_TABLE_TYPE_DEFAULT,
> -		SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY
> -	} type;
> +	u8 type:1;
As I said before. Please remove this from your patch.

> +
> +	/**
> +	 * extra_flags
> +	 * @SYSCTL_TABLE_EXTRA_PTR: flag indicating that this uses extra1/extra2.
> +	 * @SYSCTL_TABLE_EXTRA_LONG_INIT_MIN: flag indicating that this uses min/max
> +					      and min has been initialized.
> +	 * @SYSCTL_TABLE_EXTRA_LONG_INIT_MAX: flag indicating that this uses min/max
> +					      and max has been initialized.
> +	 * @SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX: flag indicating that this uses min/max
> +						 and both have been initialized.
> +	 *
> +	 */
> +	u8 extra_flags:2;
This extra_flag is needed because now you have lost the extra bit of
information that the NULL pointer gave you. This is effectively adding 2
bits per ctl_table element for all ctl_table types; event the ones that
do not need min max. So how much will we actually save with all this?
once you have added these 2 bits and removed the static variables from
the files that are not using the pointers? Is saving read only memory
the only reason for this? If that is the case, please add some
calculations of how much we save to see if it actually make sense. To
calculate the memory gains/losses you can use the bloat-o-meter script
under the scripts directory (something similar to what we did here
https://lore.kernel.org/all/20240314-jag-sysctl_remset_net-v1-0-aa26b44d29d9@samsung.com)

I'll hold off on reviewing the other patches in this set until this is a
bit more clear.

> +	union {
> +		struct {
> +			void *extra1;
> +			void *extra2;
> +		};
> +		struct {
> +			long min;
> +			long max;
> +		};
> +	};
> +
>  	proc_handler *proc_handler;	/* Callback for text formatting */
>  	struct ctl_table_poll *poll;
> -	void *extra1;
> -	void *extra2;
>  } __randomize_layout;
...

Best
-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 1/9] sysctl: support encoding values directly in the table entry

[Wen Yang]

On 2024/3/22 00:27, Joel Granados wrote:
> On Sat, Mar 09, 2024 at 06:31:18PM +0800, wenyang.linux@foxmail.com wrote:
>> From: Wen Yang <wenyang.linux@foxmail.com>
>>
>> Eric points out: "by turning .extra1 and .extra2 into longs instead of
>> keeping them as pointers and needing constants to be pointed at somewhere
>> ... The only people I can see who find a significant benefit by
>> consolidating all of the constants into one place are people who know how
>> to stomp kernel memory."
> I'm assuming that this is the "why" of the commit. Please change it so
> it is more direct. Something like "Directly encode numeric values in
> macros in order to ...". If you want to add Eric's opinion please add it
> as a link (Please follow Documentation/process/submitting-patches.rst)
> 
> 
>>
>> This patch supports encoding values directly in table entries through the
>> following work:
>> - extra1/extra2 and min/max are placed in one union to ensure that the
>>    previous code is not broken, then we have time to remove unnecessary
>>    extra1/extra2 progressively;
>> - since type only has two states, use one bit to represent it;
> Please remove this optimization from your commit. This will conflict
> with work that Thomas is doing here
> https://lore.kernel.org/all/20240222-sysctl-empty-dir-v1-0-45ba9a6352e8@weissschuh.net
> 
>> - two bits were used to represent the information of the above union( 0:
>>    using extra1/extra2, 1: using min, 2: using max, 3: using both min/max);
>> - added some helper macros.
>>
>> Suggested-by: Eric W. Biederman <ebiederm@xmission.com>
>> Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
>> Cc: Luis Chamberlain <mcgrof@kernel.org>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Joel Granados <j.granados@samsung.com>
>> Cc: Eric W. Biederman <ebiederm@xmission.com>
>> Cc: Christian Brauner <brauner@kernel.org>
>> Cc: Iurii Zaikin <yzaikin@google.com>
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>   include/linux/sysctl.h | 108 ++++++++++++++++++++++++++++++++++++++---
>>   kernel/sysctl.c        |  61 +++++++++++++++++------
>>   2 files changed, 148 insertions(+), 21 deletions(-)
>>
>> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
>> index ee7d33b89e9e..1ba980219e40 100644
>> --- a/include/linux/sysctl.h
>> +++ b/include/linux/sysctl.h
>> @@ -61,6 +61,25 @@ extern const int sysctl_vals[];
>>   
>>   extern const unsigned long sysctl_long_vals[];
>>   
>> +#define	SYSCTL_NUMERIC_NEG_ONE			((long)-1)
>> +#define	SYSCTL_NUMERIC_ZERO			(0L)
>> +#define	SYSCTL_NUMERIC_ONE			(1L)
>> +#define	SYSCTL_NUMERIC_TWO			(2L)
>> +#define	SYSCTL_NUMERIC_THREE			(3L)
>> +#define	SYSCTL_NUMERIC_FOUR			(4L)
>> +#define	SYSCTL_NUMERIC_ONE_HUNDRED		(100L)
>> +#define	SYSCTL_NUMERIC_TWO_HUNDRED		(200L)
>> +#define	SYSCTL_NUMERIC_THREE_HUNDRED		(300L)
>> +#define	SYSCTL_NUMERIC_FIVE_HUNDRED		(500L)
>> +#define	SYSCTL_NUMERIC_ONE_THOUSAND		(1000L)
>> +#define	SYSCTL_NUMERIC_TWO_THOUSAND		(2000L)
>> +#define	SYSCTL_NUMERIC_THREE_THOUSAND		(3000L)
>> +#define	SYSCTL_NUMERIC_16K			(16384L)
>> +#define	SYSCTL_NUMERIC_U8_MAX			((long)U8_MAX)
>> +#define	SYSCTL_NUMERIC_U16_MAX			((long)U16_MAX)
>> +#define	SYSCTL_NUMERIC_INT_MAX			((long)INT_MAX)
>> +#define	SYSCTL_NUMERIC_LONG_MAX			(LONG_MAX)
>> +
>>   typedef int proc_handler(struct ctl_table *ctl, int write, void *buffer,
>>   		size_t *lenp, loff_t *ppos);
>>   
>> @@ -131,6 +150,18 @@ static inline void *proc_sys_poll_event(struct ctl_table_poll *poll)
>>   #define DEFINE_CTL_TABLE_POLL(name)					\
>>   	struct ctl_table_poll name = __CTL_TABLE_POLL_INITIALIZER(name)
>>   
>> +enum {
>> +	SYSCTL_TABLE_TYPE_DEFAULT,
>> +	SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY
>> +};
>> +
>> +enum {
>> +	SYSCTL_TABLE_EXTRA_PTR,
>> +	SYSCTL_TABLE_EXTRA_LONG_INIT_MIN,
>> +	SYSCTL_TABLE_EXTRA_LONG_INIT_MAX,
>> +	SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX
>> +};
>> +
>>   /* A sysctl table is an array of struct ctl_table: */
>>   struct ctl_table {
>>   	const char *procname;		/* Text ID for /proc/sys, or zero */
>> @@ -138,20 +169,39 @@ struct ctl_table {
>>   	int maxlen;
>>   	umode_t mode;
>>   	/**
>> -	 * enum type - Enumeration to differentiate between ctl target types
>> +	 * type - Indicates to differentiate between ctl target types
>>   	 * @SYSCTL_TABLE_TYPE_DEFAULT: ctl target with no special considerations
>>   	 * @SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY: Used to identify a permanently
>>   	 *                                       empty directory target to serve
>>   	 *                                       as mount point.
>>   	 */
>> -	enum {
>> -		SYSCTL_TABLE_TYPE_DEFAULT,
>> -		SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY
>> -	} type;
>> +	u8 type:1;
> As I said before. Please remove this from your patch.
> 
>> +
>> +	/**
>> +	 * extra_flags
>> +	 * @SYSCTL_TABLE_EXTRA_PTR: flag indicating that this uses extra1/extra2.
>> +	 * @SYSCTL_TABLE_EXTRA_LONG_INIT_MIN: flag indicating that this uses min/max
>> +					      and min has been initialized.
>> +	 * @SYSCTL_TABLE_EXTRA_LONG_INIT_MAX: flag indicating that this uses min/max
>> +					      and max has been initialized.
>> +	 * @SYSCTL_TABLE_EXTRA_LONG_INIT_MINMAX: flag indicating that this uses min/max
>> +						 and both have been initialized.
>> +	 *
>> +	 */
>> +	u8 extra_flags:2;
> This extra_flag is needed because now you have lost the extra bit of
> information that the NULL pointer gave you. This is effectively adding 2
> bits per ctl_table element for all ctl_table types; event the ones that
> do not need min max. So how much will we actually save with all this?
> once you have added these 2 bits and removed the static variables from
> the files that are not using the pointers? Is saving read only memory
> the only reason for this? If that is the case, please add some
> calculations of how much we save to see if it actually make sense. To
> calculate the memory gains/losses you can use the bloat-o-meter script
> under the scripts directory (something similar to what we did here
> https://lore.kernel.org/all/20240314-jag-sysctl_remset_net-v1-0-aa26b44d29d9@samsung.com)
> 
> I'll hold off on reviewing the other patches in this set until this is a
> bit more clear.
> 

Thank you for your comments.

When we started this work, we had not yet seen Thomas's patch, so by 
borrowing the existing enum type‘s field, we can achieve directly 
encoding values without increasing the size of the ctl_table.

We really appreciate that you pointed out this issue. It will take some 
time for rework and the v3 will be sent out within a few weeks.

In addition, the patch below is not related to "kill sysctl_vals". It is 
just a regular optimization and was sent over 10 days ago. We also hope 
to receive your kind advice:

https://lkml.org/lkml/2024/3/8/871

[RESEND PATCH v2] sysctl: move the extra1/2 boundary check of u8 to 
sysctl_check_table_array

--
Best wishes,
Wen


>> +	union {
>> +		struct {
>> +			void *extra1;
>> +			void *extra2;
>> +		};
>> +		struct {
>> +			long min;
>> +			long max;
>> +		};
>> +	};
>> +
>>   	proc_handler *proc_handler;	/* Callback for text formatting */
>>   	struct ctl_table_poll *poll;
>> -	void *extra1;
>> -	void *extra2;
>>   } __randomize_layout;
> ...
> 
> Best

Re: [PATCH v2 1/9] sysctl: support encoding values directly in the table entry

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]

On Fri, Mar 22, 2024 at 11:43:16PM +0800, Wen Yang wrote:
> 
> 
<--- snip --->
> > information that the NULL pointer gave you. This is effectively adding 2
> > bits per ctl_table element for all ctl_table types; event the ones that
> > do not need min max. So how much will we actually save with all this?
> > once you have added these 2 bits and removed the static variables from
> > the files that are not using the pointers? Is saving read only memory
> > the only reason for this? If that is the case, please add some
> > calculations of how much we save to see if it actually make sense. To
> > calculate the memory gains/losses you can use the bloat-o-meter script
> > under the scripts directory (something similar to what we did here
> > https://lore.kernel.org/all/20240314-jag-sysctl_remset_net-v1-0-aa26b44d29d9@samsung.com)
> > 
> > I'll hold off on reviewing the other patches in this set until this is a
> > bit more clear.
> > 
> 
> Thank you for your comments.
> 
> When we started this work, we had not yet seen Thomas's patch, so by 
> borrowing the existing enum type‘s field, we can achieve directly 
> encoding values without increasing the size of the ctl_table.
> 
> We really appreciate that you pointed out this issue. It will take some 
> time for rework and the v3 will be sent out within a few weeks.
> 
> In addition, the patch below is not related to "kill sysctl_vals". It is 
> just a regular optimization and was sent over 10 days ago. We also hope 
> to receive your kind advice:
> 
> https://lkml.org/lkml/2024/3/8/871
> 
> [RESEND PATCH v2] sysctl: move the extra1/2 boundary check of u8 to 
> sysctl_check_table_array

Thx for pointing this out, I had missed it. On my todo list :)

> 
> --
> Best wishes,
> Wen
> 
> 
> >> +	union {
> >> +		struct {
> >> +			void *extra1;
> >> +			void *extra2;
> >> +		};
> >> +		struct {
> >> +			long min;
> >> +			long max;
> >> +		};
> >> +	};
> >> +
> >>   	proc_handler *proc_handler;	/* Callback for text formatting */
> >>   	struct ctl_table_poll *poll;
> >> -	void *extra1;
> >> -	void *extra2;
> >>   } __randomize_layout;
> > ...
> > 
> > Best
> 

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]