[PATCH v3 0/5] sysctl: encode the min/max values directly in the table entry

[PATCH v3 0/5] sysctl: encode the min/max values directly in the table entry

[Wen Yang]

Many modules use these additional static/global variables (such as
two_five_five, n_65535, ue_int_max, etc.) in the boundary checking of
sysctl, and they are read-only and never changed.

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 series achieves direct encoding values in table entries and still
maintains compatibility with existing extra1/extra2 pointers.
Afterwards, we can remove these unnecessary static variables progressively and
also gradually kill the shared const array.

Wen Yang (5):
  sysctl: add helper functions to extract table->extra1/extra2
  sysctl: support encoding values directly in the table entry
  sysctl: add KUnit test code to check for encoding  min/max in table
    entries
  sysctl: delete mmap_rnd_bits_{min/max} and
    mmap_rnd_compat_bits_{min/max} to save 16 bytes
  sysctl: delete six_hundred_forty_kb to save 4 bytes

 fs/proc/proc_sysctl.c  |  29 +-
 include/linux/mm.h     |   4 -
 include/linux/sysctl.h |  95 ++++++-
 kernel/sysctl-test.c   | 581 +++++++++++++++++++++++++++++++++++++++++
 kernel/sysctl.c        |  45 ++--
 mm/mmap.c              |   4 -
 6 files changed, 708 insertions(+), 50 deletions(-)

-- 
2.25.1

[PATCH v3 1/5] sysctl: add helper functions to extract table->extra1/extra2

[Wen Yang]

Add some sysctl helper functions to avoid direct access to
table->extra1/extra2.

Signed-off-by: Wen Yang <wen.yang@linux.dev>
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: Dave Young <dyoung@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 fs/proc/proc_sysctl.c  | 21 +++++++++------------
 include/linux/sysctl.h | 40 ++++++++++++++++++++++++++++++++++++++++
 kernel/sysctl.c        | 20 ++++++++++----------
 3 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index fac566065ed5..90c99eb1abf6 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1115,18 +1115,15 @@ static int sysctl_check_table_array(const char *path, const struct ctl_table *ta
 		if (table->maxlen != sizeof(u8))
 			err |= sysctl_err(path, table, "array not allowed");
 
-		if (table->extra1) {
-			extra = *(unsigned int *) table->extra1;
-			if (extra > 255U)
-				err |= sysctl_err(path, table,
-						"range value too large for proc_dou8vec_minmax");
-		}
-		if (table->extra2) {
-			extra = *(unsigned int *) table->extra2;
-			if (extra > 255U)
-				err |= sysctl_err(path, table,
-						"range value too large for proc_dou8vec_minmax");
-		}
+		extra = sysctl_range_min_u8(table);
+		if (extra > 255U)
+			err |= sysctl_err(path, table,
+					"range value too large for proc_dou8vec_minmax\n");
+
+		extra = sysctl_range_max_u8(table);
+		if (extra > 255U)
+			err |= sysctl_err(path, table,
+					"range value too large for proc_dou8vec_minmax\n");
 	}
 
 	if (table->proc_handler == proc_dobool) {
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 202855befa8b..20e3914ec53f 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -212,6 +212,46 @@ struct ctl_table_root {
 #define register_sysctl(path, table)	\
 	register_sysctl_sz(path, table, ARRAY_SIZE(table))
 
+static inline unsigned int sysctl_range_min_u8(const struct ctl_table *table)
+{
+	return (table->extra1) ? *(unsigned int *) table->extra1 : 0;
+}
+
+static inline unsigned int sysctl_range_max_u8(const struct ctl_table *table)
+{
+	return (table->extra2) ? *(unsigned int *) table->extra2 : U8_MAX;
+}
+
+static inline int sysctl_range_min_int(const struct ctl_table *table)
+{
+	return (table->extra1) ? *(int *) table->extra1 : INT_MIN;
+}
+
+static inline int sysctl_range_max_int(const struct ctl_table *table)
+{
+	return (table->extra2) ? *(int *) table->extra2 : INT_MAX;
+}
+
+static inline unsigned int sysctl_range_min_uint(const struct ctl_table *table)
+{
+	return (table->extra1) ? *(unsigned int *) table->extra1 : 0;
+}
+
+static inline unsigned int sysctl_range_max_uint(const struct ctl_table *table)
+{
+	return (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
+}
+
+static inline unsigned long sysctl_range_min_ulong(const struct ctl_table *table)
+{
+	return (table->extra1) ? *(unsigned long *) table->extra1 : 0;
+}
+
+static inline unsigned long sysctl_range_max_ulong(const struct ctl_table *table)
+{
+	return (table->extra2) ? *(unsigned long *) table->extra2 : ULONG_MAX;
+}
+
 #ifdef CONFIG_SYSCTL
 
 void proc_sys_poll_notify(struct ctl_table_poll *poll);
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 92305cdbb94a..86de15638e31 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -869,8 +869,8 @@ int proc_dointvec_minmax(const struct ctl_table *table, int write,
 {
 	struct proc_minmax_conv_param param;
 
-	param.min = (table->extra1) ?  *(int *) table->extra1 : INT_MIN;
-	param.max = (table->extra2) ?  *(int *) table->extra2 : INT_MAX;
+	param.min = sysctl_range_min_int(table);
+	param.max = sysctl_range_max_int(table);
 	return do_proc_dointvec(table, write, buffer, lenp, ppos,
 				do_proc_dointvec_minmax_conv, &param);
 }
@@ -923,8 +923,8 @@ int proc_douintvec_minmax(const struct ctl_table *table, int write,
 {
 	struct proc_minmax_conv_param param;
 
-	param.min = (table->extra1) ? *(unsigned int *) table->extra1 : 0;
-	param.max = (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
+	param.min = sysctl_range_min_uint(table);
+	param.max = sysctl_range_max_uint(table);
 	return do_proc_douintvec(table, write, buffer, lenp, ppos,
 				 do_proc_douintvec_minmax_conv, &param);
 }
@@ -959,8 +959,8 @@ int proc_dou8vec_minmax(const struct ctl_table *table, int write,
 	if (table->maxlen != sizeof(u8))
 		return -EINVAL;
 
-	param.min = (table->extra1) ? *(unsigned int *) table->extra1 : 0;
-	param.max = (table->extra2) ? *(unsigned int *) table->extra2 : 255U;
+	param.min = sysctl_range_min_u8(table);
+	param.max = sysctl_range_max_u8(table);
 	tmp = *table;
 
 	tmp.maxlen = sizeof(val);
@@ -1012,8 +1012,8 @@ static int __do_proc_doulongvec_minmax(void *data,
 	}
 
 	i = data;
-	min = (table->extra1) ? *(unsigned long *) table->extra1 : 0;
-	max = (table->extra2) ? *(unsigned long *) table->extra2 : ULONG_MAX;
+	min = sysctl_range_min_ulong(table);
+	max = sysctl_range_max_ulong(table);
 
 	vleft = table->maxlen / sizeof(unsigned long);
 	left = *lenp;
@@ -1250,8 +1250,8 @@ int proc_dointvec_ms_jiffies_minmax(const struct ctl_table *table, int write,
 {
 	struct proc_minmax_conv_param param;
 
-	param.min =  (table->extra1) ? *(int *) table->extra1 : INT_MIN;
-	param.max =  (table->extra2) ? *(int *) table->extra2 : INT_MAX;
+	param.min = sysctl_range_min_int(table);
+	param.max = sysctl_range_max_int(table);
 	return do_proc_dointvec(table, write, buffer, lenp, ppos,
 			do_proc_dointvec_ms_jiffies_minmax_conv, &param);
 }
-- 
2.25.1

[PATCH v3 2/5] sysctl: support encoding values directly in the table entry

[Wen Yang]

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 gradually remove these
  unnecessary extra1/extra2;
- two bits were used to represent the information of the above union:
  SYSCTL_FLAG_MIN: 0, using extra1. 1, using min.
  SYSCTL_FLAG_MAX: 0, using extra2. 1, using max.
- since the proc file's mode field only uses 9 bits(777), we could use the
  additional two bits(S_ISUID and S_ISGID) to temporarily represent
  SYSCTL_FLAG_MIN and SYSCTL_FLAG_MAX.
- added some helper macros.

By introducing long min/max to replace void * extra1/extra2 in most cases,
unnecessary variables can be removed to save memory and avoid attacks.

Signed-off-by: Wen Yang <wen.yang@linux.dev>
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: Dave Young <dyoung@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 fs/proc/proc_sysctl.c  |  8 +++--
 include/linux/sysctl.h | 71 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 67 insertions(+), 12 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 90c99eb1abf6..e88d1dca2a80 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -848,8 +848,11 @@ static int proc_sys_getattr(struct mnt_idmap *idmap,
 		return PTR_ERR(head);
 
 	generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
-	if (table)
+	if (table) {
 		stat->mode = (stat->mode & S_IFMT) | table->mode;
+		stat->mode &= ~SYSCTL_FLAG_MIN;
+		stat->mode &= ~SYSCTL_FLAG_MAX;
+	}
 
 	sysctl_head_finish(head);
 	return 0;
@@ -1163,7 +1166,8 @@ static int sysctl_check_table(const char *path, struct ctl_table_header *header)
 		if (!entry->proc_handler)
 			err |= sysctl_err(path, entry, "No proc_handler");
 
-		if ((entry->mode & (S_IRUGO|S_IWUGO)) != entry->mode)
+		if ((entry->mode & (S_IRUGO|S_IWUGO|SYSCTL_FLAG_MIN|SYSCTL_FLAG_MAX))
+		    != entry->mode)
 			err |= sysctl_err(path, entry, "bogus .mode 0%o",
 				entry->mode);
 	}
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 20e3914ec53f..8e27e8350ca8 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -28,6 +28,7 @@
 #include <linux/rbtree.h>
 #include <linux/uidgid.h>
 #include <uapi/linux/sysctl.h>
+#include <uapi/linux/stat.h>
 
 /* For the /proc/sys support */
 struct completion;
@@ -61,6 +62,37 @@ extern const int sysctl_vals[];
 
 extern const unsigned long sysctl_long_vals[];
 
+#define SYSCTL_NUM_ZERO                         (0L)
+#define SYSCTL_NUM_ONE                          (1L)
+#define SYSCTL_NUM_TWO                          (2L)
+#define SYSCTL_NUM_THREE                        (3L)
+#define SYSCTL_NUM_FOUR                         (4L)
+#define SYSCTL_NUM_FIVE                         (5L)
+#define SYSCTL_NUM_SIX                          (6L)
+#define SYSCTL_NUM_SEVEN                        (7L)
+#define SYSCTL_NUM_EIGHT                        (8L)
+#define SYSCTL_NUM_NINE                         (9L)
+#define SYSCTL_NUM_TEN                          (10L)
+#define SYSCTL_NUM_SIXTEEN                      (16L)
+#define SYSCTL_NUM_THIRTY_ONE                   (31L)
+#define SYSCTL_NUM_NEG_THIRTY_ONE               (-31L)
+#define SYSCTL_NUM_ONE_HUNDRED                  (100L)
+#define SYSCTL_NUM_TWO_HUNDRED                  (200L)
+#define SYSCTL_NUM_S8_MAX                       (127L)
+#define SYSCTL_NUM_U8_MAX                       (255L)
+#define SYSCTL_NUM_FIVE_HUNDRED                 (500L)
+#define SYSCTL_NUM_ONE_THOUSAND                 (1000L)
+#define SYSCTL_NUM_THREE_THOUSAND               (3000L)
+#define SYSCTL_NUM_16K                          (16 * 1024L)
+#define SYSCTL_NUM_16M                          (16 * 1024 * 1024L)
+#define SYSCTL_NUM_SEC_PER_HOUR                 (60 * 60L)
+#define SYSCTL_NUM_U16_MAX                      (65535L)
+#define SYSCTL_NUM_SEC_PER_DAY                  (24 * 60 * 60L)
+#define SYSCTL_NUM_MS_PER_DAY                   (24 * 60 * 60 * 1000L)
+#define SYSCTL_NUM_INT_MAX                      (INT_MAX)
+#define SYSCTL_NUM_NEG_ONE                      (-1)
+#define SYSCTL_NUM_LONG_MAX                     (LONG_MAX)
+
 typedef int proc_handler(const struct ctl_table *ctl, int write, void *buffer,
 		size_t *lenp, loff_t *ppos);
 
@@ -131,6 +163,9 @@ 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)
 
+#define  SYSCTL_FLAG_MIN			S_ISUID
+#define  SYSCTL_FLAG_MAX			S_ISGID
+
 /* A sysctl table is an array of struct ctl_table: */
 struct ctl_table {
 	const char *procname;		/* Text ID for /proc/sys, or zero */
@@ -139,8 +174,16 @@ struct ctl_table {
 	umode_t mode;
 	proc_handler *proc_handler;	/* Callback for text formatting */
 	struct ctl_table_poll *poll;
-	void *extra1;
-	void *extra2;
+	union {
+		struct {
+			void *extra1;
+			void *extra2;
+		};
+		struct {
+			long min;
+			long max;
+		};
+	};
 } __randomize_layout;
 
 struct ctl_node {
@@ -214,42 +257,50 @@ struct ctl_table_root {
 
 static inline unsigned int sysctl_range_min_u8(const struct ctl_table *table)
 {
-	return (table->extra1) ? *(unsigned int *) table->extra1 : 0;
+	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
+	       (table->extra1) ? *(unsigned int *) table->extra1 : 0;
 }
 
 static inline unsigned int sysctl_range_max_u8(const struct ctl_table *table)
 {
-	return (table->extra2) ? *(unsigned int *) table->extra2 : U8_MAX;
+	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
+	       (table->extra2) ? *(unsigned int *) table->extra2 : U8_MAX;
 }
 
 static inline int sysctl_range_min_int(const struct ctl_table *table)
 {
-	return (table->extra1) ? *(int *) table->extra1 : INT_MIN;
+	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
+	       (table->extra1) ? *(int *) table->extra1 : INT_MIN;
 }
 
 static inline int sysctl_range_max_int(const struct ctl_table *table)
 {
-	return (table->extra2) ? *(int *) table->extra2 : INT_MAX;
+	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
+	       (table->extra2) ? *(int *) table->extra2 : INT_MAX;
 }
 
 static inline unsigned int sysctl_range_min_uint(const struct ctl_table *table)
 {
-	return (table->extra1) ? *(unsigned int *) table->extra1 : 0;
+	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
+	       (table->extra1) ? *(unsigned int *) table->extra1 : 0;
 }
 
 static inline unsigned int sysctl_range_max_uint(const struct ctl_table *table)
 {
-	return (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
+	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
+	       (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
 }
 
 static inline unsigned long sysctl_range_min_ulong(const struct ctl_table *table)
 {
-	return (table->extra1) ? *(unsigned long *) table->extra1 : 0;
+	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
+	       (table->extra1) ? *(unsigned long *) table->extra1 : 0;
 }
 
 static inline unsigned long sysctl_range_max_ulong(const struct ctl_table *table)
 {
-	return (table->extra2) ? *(unsigned long *) table->extra2 : ULONG_MAX;
+	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
+	       (table->extra2) ? *(unsigned long *) table->extra2 : ULONG_MAX;
 }
 
 #ifdef CONFIG_SYSCTL
-- 
2.25.1

[PATCH v3 3/5] sysctl: add KUnit test code to check for encoding min/max in table entries

[Wen Yang]

Add KUnit test code to check the impact of encoding the min/max values
directly in table entries on functions such as proc_rointvec,
proc_rou8vectminmax, proc_rouintvectminmax, and proc_roulongvectminmax,
including basic parsing testing and min/max overflow testing.

Signed-off-by: Wen Yang <wen.yang@linux.dev>
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: Dave Young <dyoung@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 kernel/sysctl-test.c | 581 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 581 insertions(+)

diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
index 3ac98bb7fb82..486240787103 100644
--- a/kernel/sysctl-test.c
+++ b/kernel/sysctl-test.c
@@ -415,6 +415,575 @@ static void sysctl_test_register_sysctl_sz_invalid_extra_value(
 	KUNIT_EXPECT_NOT_NULL(test, register_sysctl("foo", table_qux));
 }
 
+static void sysctl_test_api_dointvec_write_with_minmax(
+		struct kunit *test)
+{
+	int data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(int),
+		.mode		= 0644 | SYSCTL_FLAG_MIN | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_dointvec_minmax,
+		.min		= SYSCTL_NUM_NEG_ONE,
+		.max		= SYSCTL_NUM_ONE_HUNDRED,
+	};
+	size_t max_len = 32, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "10";
+	char input2[] = "-5";
+	char input3[] = "200";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 10, *((int *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 10, *((int *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 10, *((int *)table.data));
+}
+
+static void sysctl_test_api_dointvec_write_with_min(
+		struct kunit *test)
+{
+	int data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(int),
+		.mode		= 0644 | SYSCTL_FLAG_MIN,
+		.proc_handler	= proc_dointvec_minmax,
+		.min		= SYSCTL_NUM_NEG_ONE,
+	};
+	size_t max_len = 32, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "10";
+	char input2[] = "-5";
+	char input3[] = "2147483647";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 10, *((int *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 10, *((int *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input3) - 1, len);
+	KUNIT_EXPECT_EQ(test, 2147483647, *((int *)table.data));
+}
+
+static void sysctl_test_api_dointvec_write_with_max(
+		struct kunit *test)
+{
+	int data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(int),
+		.mode		= 0644 | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_dointvec_minmax,
+		.max		= SYSCTL_NUM_ONE_HUNDRED,
+	};
+	size_t max_len = 32, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "10";
+	char input2[] = "2147483647";
+	char input3[] = "-2147483648";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 10, *((int *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 10, *((int *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_dointvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input3) - 1, len);
+	KUNIT_EXPECT_EQ(test, -2147483648, *((int *)table.data));
+}
+
+static void sysctl_test_api_douintvec_write_with_minmax(
+		struct kunit *test)
+{
+	unsigned int data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(int),
+		.mode		= 0644 | SYSCTL_FLAG_MIN | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_douintvec_minmax,
+		.min		= SYSCTL_NUM_FOUR,
+		.max		= SYSCTL_NUM_TWO_HUNDRED,
+	};
+	size_t max_len = 32, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "100";
+	char input2[] = "3";
+	char input3[] = "1000";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 100, *((unsigned int *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 100, *((unsigned int *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 100, *((unsigned int *)table.data));
+}
+
+static void sysctl_test_api_douintvec_write_with_min(
+		struct kunit *test)
+{
+	unsigned int data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(int),
+		.mode		= 0644 | SYSCTL_FLAG_MIN,
+		.proc_handler	= proc_douintvec_minmax,
+		.min		= SYSCTL_NUM_FOUR,
+	};
+	size_t max_len = 32, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "100";
+	char input2[] = "3";
+	char input3[] = "4294967295";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 100, *((unsigned int *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 100, *((unsigned int *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input3) - 1, len);
+	KUNIT_EXPECT_EQ(test, 4294967295, *((unsigned int *)table.data));
+}
+
+static void sysctl_test_api_douintvec_write_with_max(
+		struct kunit *test)
+{
+	unsigned int data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(int),
+		.mode		= 0644 | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_douintvec_minmax,
+		.max		= SYSCTL_NUM_ONE_THOUSAND,
+	};
+	size_t max_len = 32, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "900";
+	char input2[] = "10000";
+	char input3[] = "0";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 900, *((unsigned int *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 900, *((unsigned int *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_douintvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input3) - 1, len);
+	KUNIT_EXPECT_EQ(test, 0, *((unsigned int *)table.data));
+}
+
+static void sysctl_test_api_dou8vec_write_with_minmax(
+		struct kunit *test)
+{
+	unsigned char data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(unsigned char),
+		.mode		= 0644 | SYSCTL_FLAG_MIN | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_dou8vec_minmax,
+		.min		= SYSCTL_NUM_THREE,
+		.max		= SYSCTL_NUM_ONE_HUNDRED,
+	};
+	size_t max_len = 8, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "32";
+	char input2[] = "1";
+	char input3[] = "200";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_dou8vec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 32, *((unsigned char *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_dou8vec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 32, *((unsigned char *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_dou8vec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 32, *((unsigned char *)table.data));
+}
+
+static void sysctl_test_api_dou8vec_write_with_min(
+		struct kunit *test)
+{
+	unsigned char data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(unsigned char),
+		.mode		= 0644 | SYSCTL_FLAG_MIN,
+		.proc_handler	= proc_dou8vec_minmax,
+		.min		= SYSCTL_NUM_THREE,
+	};
+	size_t max_len = 8, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "32";
+	char input2[] = "1";
+	char input3[] = "255";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_dou8vec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 32, *((unsigned char *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_dou8vec_minmax(&table, KUNIT_PROC_WRITE,
+			user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 32, *((unsigned char *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_dou8vec_minmax(&table, KUNIT_PROC_WRITE,
+			user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input3) - 1, len);
+	KUNIT_EXPECT_EQ(test, 255, *((unsigned char *)table.data));
+}
+
+static void sysctl_test_api_dou8vec_write_with_max(
+		struct kunit *test)
+{
+	unsigned char data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(unsigned char),
+		.mode		= 0644 | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_dou8vec_minmax,
+		.max		= SYSCTL_NUM_TWO_HUNDRED,
+	};
+	size_t max_len = 8, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "32";
+	char input2[] = "0";
+	char input3[] = "255";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_dou8vec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 32, *((unsigned char *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_dou8vec_minmax(&table, KUNIT_PROC_WRITE,
+			user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input2) - 1, len);
+	KUNIT_EXPECT_EQ(test, 0, *((unsigned char *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_dou8vec_minmax(&table, KUNIT_PROC_WRITE,
+			user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 0, *((unsigned char *)table.data));
+}
+
+static void sysctl_test_api_doulongvec_write_with_minmax(
+		struct kunit *test)
+{
+	unsigned long data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(unsigned long),
+		.mode		= 0644 | SYSCTL_FLAG_MIN | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_doulongvec_minmax,
+		.min		= SYSCTL_NUM_ONE_THOUSAND,
+		.max		= SYSCTL_NUM_THREE_THOUSAND,
+	};
+	size_t max_len = 64, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "1024";
+	char input2[] = "100";
+	char input3[] = "4096";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_doulongvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 1024, *((unsigned long *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_doulongvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 1024, *((unsigned long *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_doulongvec_minmax(&table, KUNIT_PROC_WRITE,
+				user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 1024, *((unsigned long *)table.data));
+}
+
+static void sysctl_test_api_doulongvec_write_with_min(
+		struct kunit *test)
+{
+	unsigned long data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(unsigned long),
+		.mode		= 0644 | SYSCTL_FLAG_MIN,
+		.proc_handler	= proc_doulongvec_minmax,
+		.min		= SYSCTL_NUM_ONE_THOUSAND,
+	};
+	size_t max_len = 64, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "1000";
+	char input2[] = "10";
+	char input3[64] = {0};
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_doulongvec_minmax(&table, KUNIT_PROC_WRITE,
+			user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 1000, *((unsigned long *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_doulongvec_minmax(&table, KUNIT_PROC_WRITE,
+			user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 1000, *((unsigned long *)table.data));
+
+	// test for input3
+	snprintf(input3, sizeof(input3), "%lu", ULONG_MAX);
+	len = strlen(input3);
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_doulongvec_minmax(&table, KUNIT_PROC_WRITE,
+			user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, strlen(input3), len);
+	KUNIT_EXPECT_EQ(test, ULONG_MAX, *((unsigned long *)table.data));
+}
+
+static void sysctl_test_api_doulongvec_write_with_max(
+		struct kunit *test)
+{
+	unsigned long data = 0;
+	struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(unsigned long),
+		.mode		= 0644 | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_doulongvec_minmax,
+		.max		= SYSCTL_NUM_THREE_THOUSAND,
+	};
+	size_t max_len = 64, len = max_len;
+	loff_t pos = 0;
+	char *buffer = kunit_kzalloc(test, max_len, GFP_USER);
+	char __user *user_buffer = (char __user *)buffer;
+	char input1[] = "1024";
+	char input2[] = "4096";
+	char input3[] = "0";
+
+	// test for input1
+	len = sizeof(input1) - 1;
+	memcpy(buffer, input1, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_doulongvec_minmax(&table, KUNIT_PROC_WRITE,
+			user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input1) - 1, len);
+	KUNIT_EXPECT_EQ(test, 1024, *((unsigned long *)table.data));
+
+	// test for input2
+	len = sizeof(input2) - 1;
+	pos = 0;
+	memcpy(buffer, input2, len);
+	KUNIT_EXPECT_EQ(test, -EINVAL, proc_doulongvec_minmax(&table, KUNIT_PROC_WRITE,
+			user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, 0, pos);
+	KUNIT_EXPECT_EQ(test, 1024, *((unsigned long *)table.data));
+
+	// test for input3
+	len = sizeof(input3) - 1;
+	pos = 0;
+	memcpy(buffer, input3, len);
+	KUNIT_EXPECT_EQ(test, 0, proc_doulongvec_minmax(&table, KUNIT_PROC_WRITE,
+			user_buffer, &len, &pos));
+	KUNIT_EXPECT_EQ(test, sizeof(input3) - 1, len);
+	KUNIT_EXPECT_EQ(test, 0, *((unsigned long *)table.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),
@@ -427,6 +996,18 @@ static struct kunit_case sysctl_test_cases[] = {
 	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_register_sysctl_sz_invalid_extra_value),
+	KUNIT_CASE(sysctl_test_api_dointvec_write_with_minmax),
+	KUNIT_CASE(sysctl_test_api_dointvec_write_with_min),
+	KUNIT_CASE(sysctl_test_api_dointvec_write_with_max),
+	KUNIT_CASE(sysctl_test_api_douintvec_write_with_minmax),
+	KUNIT_CASE(sysctl_test_api_douintvec_write_with_min),
+	KUNIT_CASE(sysctl_test_api_douintvec_write_with_max),
+	KUNIT_CASE(sysctl_test_api_dou8vec_write_with_minmax),
+	KUNIT_CASE(sysctl_test_api_dou8vec_write_with_min),
+	KUNIT_CASE(sysctl_test_api_dou8vec_write_with_max),
+	KUNIT_CASE(sysctl_test_api_doulongvec_write_with_minmax),
+	KUNIT_CASE(sysctl_test_api_doulongvec_write_with_min),
+	KUNIT_CASE(sysctl_test_api_doulongvec_write_with_max),
 	{}
 };
 
-- 
2.25.1

[PATCH v3 4/5] sysctl: delete mmap_rnd_bits_{min/max} and mmap_rnd_compat_bits_{min/max} to save 16 bytes

[Wen Yang]

By directly encoding CONFIG_ARCH_MMAP_RND_BITS_{MIN/MAX} and
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_{MIN/MAX} into the ctl_table's min/max
field, unnecessary global variables can be removed, saving 16 bytes.

Signed-off-by: Wen Yang <wen.yang@linux.dev>
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: Dave Young <dyoung@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 include/linux/mm.h |  4 ----
 kernel/sysctl.c    | 12 ++++++------
 mm/mmap.c          |  4 ----
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6549d0979b28..9d9c4a4f4708 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -87,13 +87,9 @@ extern int sysctl_legacy_va_layout;
 #endif
 
 #ifdef CONFIG_HAVE_ARCH_MMAP_RND_BITS
-extern const int mmap_rnd_bits_min;
-extern int mmap_rnd_bits_max __ro_after_init;
 extern int mmap_rnd_bits __read_mostly;
 #endif
 #ifdef CONFIG_HAVE_ARCH_MMAP_RND_COMPAT_BITS
-extern const int mmap_rnd_compat_bits_min;
-extern const int mmap_rnd_compat_bits_max;
 extern int mmap_rnd_compat_bits __read_mostly;
 #endif
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 86de15638e31..05197d46007d 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2196,10 +2196,10 @@ static struct ctl_table vm_table[] = {
 		.procname	= "mmap_rnd_bits",
 		.data		= &mmap_rnd_bits,
 		.maxlen		= sizeof(mmap_rnd_bits),
-		.mode		= 0600,
+		.mode		= 0600 | SYSCTL_FLAG_MIN | SYSCTL_FLAG_MAX,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= (void *)&mmap_rnd_bits_min,
-		.extra2		= (void *)&mmap_rnd_bits_max,
+		.min		= CONFIG_ARCH_MMAP_RND_BITS_MIN,
+		.max		= CONFIG_ARCH_MMAP_RND_BITS_MAX,
 	},
 #endif
 #ifdef CONFIG_HAVE_ARCH_MMAP_RND_COMPAT_BITS
@@ -2207,10 +2207,10 @@ static struct ctl_table vm_table[] = {
 		.procname	= "mmap_rnd_compat_bits",
 		.data		= &mmap_rnd_compat_bits,
 		.maxlen		= sizeof(mmap_rnd_compat_bits),
-		.mode		= 0600,
+		.mode		= 0600 | SYSCTL_FLAG_MIN | SYSCTL_FLAG_MAX,
 		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= (void *)&mmap_rnd_compat_bits_min,
-		.extra2		= (void *)&mmap_rnd_compat_bits_max,
+		.min		= CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN,
+		.max		= CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX,
 	},
 #endif
 };
diff --git a/mm/mmap.c b/mm/mmap.c
index d0dfc85b209b..8ed6f0d31d95 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -63,13 +63,9 @@
 #endif
 
 #ifdef CONFIG_HAVE_ARCH_MMAP_RND_BITS
-const int mmap_rnd_bits_min = CONFIG_ARCH_MMAP_RND_BITS_MIN;
-int mmap_rnd_bits_max __ro_after_init = CONFIG_ARCH_MMAP_RND_BITS_MAX;
 int mmap_rnd_bits __read_mostly = CONFIG_ARCH_MMAP_RND_BITS;
 #endif
 #ifdef CONFIG_HAVE_ARCH_MMAP_RND_COMPAT_BITS
-const int mmap_rnd_compat_bits_min = CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN;
-const int mmap_rnd_compat_bits_max = CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX;
 int mmap_rnd_compat_bits __read_mostly = CONFIG_ARCH_MMAP_RND_COMPAT_BITS;
 #endif
 
-- 
2.25.1

[PATCH v3 5/5] sysctl: delete six_hundred_forty_kb to save 4 bytes

[Wen Yang]

By directly encoding specific numbers into the min/max field,
unnecessary global variable six_hundred_forty_kb can be removed,
saving 4 bytes

Signed-off-by: Wen Yang <wen.yang@linux.dev>
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: Dave Young <dyoung@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 kernel/sysctl.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 05197d46007d..7affb1ad7065 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -90,13 +90,6 @@ EXPORT_SYMBOL_GPL(sysctl_long_vals);
 
 #if defined(CONFIG_SYSCTL)
 
-/* Constants used for minimum and maximum */
-
-#ifdef CONFIG_PERF_EVENTS
-static const int six_hundred_forty_kb = 640 * 1024;
-#endif
-
-
 static const int ngroups_max = NGROUPS_MAX;
 static const int cap_last_cap = CAP_LAST_CAP;
 
@@ -1964,10 +1957,10 @@ static struct ctl_table kern_table[] = {
 		.procname	= "perf_event_max_stack",
 		.data		= &sysctl_perf_event_max_stack,
 		.maxlen		= sizeof(sysctl_perf_event_max_stack),
-		.mode		= 0644,
+		.mode		= 0644 | SYSCTL_FLAG_MIN | SYSCTL_FLAG_MAX,
 		.proc_handler	= perf_event_max_stack_handler,
-		.extra1		= SYSCTL_ZERO,
-		.extra2		= (void *)&six_hundred_forty_kb,
+		.min		= SYSCTL_NUM_ZERO,
+		.max		= 640 * 1024,
 	},
 	{
 		.procname	= "perf_event_max_contexts_per_stack",
-- 
2.25.1

Re: [PATCH v3 0/5] sysctl: encode the min/max values directly in the table entry

[Thomas Weißschuh]

Hi,

On 2024-09-15 10:08:26+0000, Wen Yang wrote:
> Many modules use these additional static/global variables (such as
> two_five_five, n_65535, ue_int_max, etc.) in the boundary checking of
> sysctl, and they are read-only and never changed.
> 
> 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 series achieves direct encoding values in table entries and still
> maintains compatibility with existing extra1/extra2 pointers.
> Afterwards, we can remove these unnecessary static variables progressively and
> also gradually kill the shared const array.

As this is already v3 it would be nice to have a series changelog and
links to previous revisions.

> 
> Wen Yang (5):
>   sysctl: add helper functions to extract table->extra1/extra2
>   sysctl: support encoding values directly in the table entry
>   sysctl: add KUnit test code to check for encoding  min/max in table
>     entries
>   sysctl: delete mmap_rnd_bits_{min/max} and
>     mmap_rnd_compat_bits_{min/max} to save 16 bytes
>   sysctl: delete six_hundred_forty_kb to save 4 bytes
> 
>  fs/proc/proc_sysctl.c  |  29 +-
>  include/linux/mm.h     |   4 -
>  include/linux/sysctl.h |  95 ++++++-
>  kernel/sysctl-test.c   | 581 +++++++++++++++++++++++++++++++++++++++++
>  kernel/sysctl.c        |  45 ++--
>  mm/mmap.c              |   4 -
>  6 files changed, 708 insertions(+), 50 deletions(-)
> 
> -- 
> 2.25.1
> 

Re: [PATCH v3 2/5] sysctl: support encoding values directly in the table entry

[Thomas Weißschuh]

On 2024-09-15 10:08:28+0000, Wen Yang wrote:
> 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 gradually remove these
>   unnecessary extra1/extra2;

The union shouldn't be necessary for backward compatibility.
It could just as well be a struct.
The union however saves some space.

> - two bits were used to represent the information of the above union:
>   SYSCTL_FLAG_MIN: 0, using extra1. 1, using min.
>   SYSCTL_FLAG_MAX: 0, using extra2. 1, using max.
> - since the proc file's mode field only uses 9 bits(777), we could use the
>   additional two bits(S_ISUID and S_ISGID) to temporarily represent
>   SYSCTL_FLAG_MIN and SYSCTL_FLAG_MAX.
> - added some helper macros.
> 
> By introducing long min/max to replace void * extra1/extra2 in most cases,
> unnecessary variables can be removed to save memory and avoid attacks.
> 
> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> 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: Dave Young <dyoung@redhat.com>
> Cc: linux-kernel@vger.kernel.org
> ---
>  fs/proc/proc_sysctl.c  |  8 +++--
>  include/linux/sysctl.h | 71 ++++++++++++++++++++++++++++++++++++------
>  2 files changed, 67 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 90c99eb1abf6..e88d1dca2a80 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -848,8 +848,11 @@ static int proc_sys_getattr(struct mnt_idmap *idmap,
>  		return PTR_ERR(head);
>  
>  	generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
> -	if (table)
> +	if (table) {
>  		stat->mode = (stat->mode & S_IFMT) | table->mode;
> +		stat->mode &= ~SYSCTL_FLAG_MIN;
> +		stat->mode &= ~SYSCTL_FLAG_MAX;
> +	}
>  
>  	sysctl_head_finish(head);
>  	return 0;
> @@ -1163,7 +1166,8 @@ static int sysctl_check_table(const char *path, struct ctl_table_header *header)
>  		if (!entry->proc_handler)
>  			err |= sysctl_err(path, entry, "No proc_handler");
>  
> -		if ((entry->mode & (S_IRUGO|S_IWUGO)) != entry->mode)
> +		if ((entry->mode & (S_IRUGO|S_IWUGO|SYSCTL_FLAG_MIN|SYSCTL_FLAG_MAX))
> +		    != entry->mode)
>  			err |= sysctl_err(path, entry, "bogus .mode 0%o",
>  				entry->mode);
>  	}
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index 20e3914ec53f..8e27e8350ca8 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -28,6 +28,7 @@
>  #include <linux/rbtree.h>
>  #include <linux/uidgid.h>
>  #include <uapi/linux/sysctl.h>
> +#include <uapi/linux/stat.h>
>  
>  /* For the /proc/sys support */
>  struct completion;
> @@ -61,6 +62,37 @@ extern const int sysctl_vals[];
>  
>  extern const unsigned long sysctl_long_vals[];
>  
> +#define SYSCTL_NUM_ZERO                         (0L)
> +#define SYSCTL_NUM_ONE                          (1L)
> +#define SYSCTL_NUM_TWO                          (2L)
> +#define SYSCTL_NUM_THREE                        (3L)
> +#define SYSCTL_NUM_FOUR                         (4L)
> +#define SYSCTL_NUM_FIVE                         (5L)
> +#define SYSCTL_NUM_SIX                          (6L)
> +#define SYSCTL_NUM_SEVEN                        (7L)
> +#define SYSCTL_NUM_EIGHT                        (8L)
> +#define SYSCTL_NUM_NINE                         (9L)
> +#define SYSCTL_NUM_TEN                          (10L)
> +#define SYSCTL_NUM_SIXTEEN                      (16L)
> +#define SYSCTL_NUM_THIRTY_ONE                   (31L)
> +#define SYSCTL_NUM_NEG_THIRTY_ONE               (-31L)
> +#define SYSCTL_NUM_ONE_HUNDRED                  (100L)
> +#define SYSCTL_NUM_TWO_HUNDRED                  (200L)
> +#define SYSCTL_NUM_S8_MAX                       (127L)
> +#define SYSCTL_NUM_U8_MAX                       (255L)
> +#define SYSCTL_NUM_FIVE_HUNDRED                 (500L)
> +#define SYSCTL_NUM_ONE_THOUSAND                 (1000L)
> +#define SYSCTL_NUM_THREE_THOUSAND               (3000L)
> +#define SYSCTL_NUM_16K                          (16 * 1024L)
> +#define SYSCTL_NUM_16M                          (16 * 1024 * 1024L)
> +#define SYSCTL_NUM_SEC_PER_HOUR                 (60 * 60L)
> +#define SYSCTL_NUM_U16_MAX                      (65535L)
> +#define SYSCTL_NUM_SEC_PER_DAY                  (24 * 60 * 60L)
> +#define SYSCTL_NUM_MS_PER_DAY                   (24 * 60 * 60 * 1000L)
> +#define SYSCTL_NUM_INT_MAX                      (INT_MAX)
> +#define SYSCTL_NUM_NEG_ONE                      (-1)
> +#define SYSCTL_NUM_LONG_MAX                     (LONG_MAX)

Are all those constants really needed?
In the past they were useful as they referenced the array of constants.
But now people can just use the number directly, or even better a
subsystem-specific constant.

> +
>  typedef int proc_handler(const struct ctl_table *ctl, int write, void *buffer,
>  		size_t *lenp, loff_t *ppos);
>  
> @@ -131,6 +163,9 @@ 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)
>  
> +#define  SYSCTL_FLAG_MIN			S_ISUID
> +#define  SYSCTL_FLAG_MAX			S_ISGID

Is it ever useful to have only one of these flags?
IMO this could be a single flag.

> +
>  /* A sysctl table is an array of struct ctl_table: */
>  struct ctl_table {
>  	const char *procname;		/* Text ID for /proc/sys, or zero */
> @@ -139,8 +174,16 @@ struct ctl_table {
>  	umode_t mode;
>  	proc_handler *proc_handler;	/* Callback for text formatting */
>  	struct ctl_table_poll *poll;
> -	void *extra1;
> -	void *extra2;
> +	union {
> +		struct {
> +			void *extra1;
> +			void *extra2;
> +		};
> +		struct {
> +			long min;
> +			long max;
> +		};
> +	};
>  } __randomize_layout;
>  
>  struct ctl_node {
> @@ -214,42 +257,50 @@ struct ctl_table_root {
>  
>  static inline unsigned int sysctl_range_min_u8(const struct ctl_table *table)
>  {
> -	return (table->extra1) ? *(unsigned int *) table->extra1 : 0;
> +	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
> +	       (table->extra1) ? *(unsigned int *) table->extra1 : 0;

All these repetitive functions could probably be replaced by one macro.
Keeping the users more direct and avoiding all the repetition.

>  }
>  
>  static inline unsigned int sysctl_range_max_u8(const struct ctl_table *table)
>  {
> -	return (table->extra2) ? *(unsigned int *) table->extra2 : U8_MAX;
> +	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
> +	       (table->extra2) ? *(unsigned int *) table->extra2 : U8_MAX;
>  }
>  
>  static inline int sysctl_range_min_int(const struct ctl_table *table)
>  {
> -	return (table->extra1) ? *(int *) table->extra1 : INT_MIN;
> +	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
> +	       (table->extra1) ? *(int *) table->extra1 : INT_MIN;
>  }
>  
>  static inline int sysctl_range_max_int(const struct ctl_table *table)
>  {
> -	return (table->extra2) ? *(int *) table->extra2 : INT_MAX;
> +	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
> +	       (table->extra2) ? *(int *) table->extra2 : INT_MAX;
>  }
>  
>  static inline unsigned int sysctl_range_min_uint(const struct ctl_table *table)
>  {
> -	return (table->extra1) ? *(unsigned int *) table->extra1 : 0;
> +	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
> +	       (table->extra1) ? *(unsigned int *) table->extra1 : 0;
>  }
>  
>  static inline unsigned int sysctl_range_max_uint(const struct ctl_table *table)
>  {
> -	return (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
> +	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
> +	       (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
>  }
>  
>  static inline unsigned long sysctl_range_min_ulong(const struct ctl_table *table)
>  {
> -	return (table->extra1) ? *(unsigned long *) table->extra1 : 0;
> +	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
> +	       (table->extra1) ? *(unsigned long *) table->extra1 : 0;
>  }
>  
>  static inline unsigned long sysctl_range_max_ulong(const struct ctl_table *table)
>  {
> -	return (table->extra2) ? *(unsigned long *) table->extra2 : ULONG_MAX;
> +	return (table->mode & SYSCTL_FLAG_MAX) ? table->max :
> +	       (table->extra2) ? *(unsigned long *) table->extra2 : ULONG_MAX;
>  }
>  
>  #ifdef CONFIG_SYSCTL
> -- 
> 2.25.1
> 

Re: [PATCH v3 2/5] sysctl: support encoding values directly in the table entry

[Wen Yang]

On 2024/9/15 14:25, Thomas Weißschuh wrote:
> On 2024-09-15 10:08:28+0000, Wen Yang wrote:
>> 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 gradually remove these
>>    unnecessary extra1/extra2;
> 
> The union shouldn't be necessary for backward compatibility.
> It could just as well be a struct.
> The union however saves some space.
> 
>> - two bits were used to represent the information of the above union:
>>    SYSCTL_FLAG_MIN: 0, using extra1. 1, using min.
>>    SYSCTL_FLAG_MAX: 0, using extra2. 1, using max.
>> - since the proc file's mode field only uses 9 bits(777), we could use the
>>    additional two bits(S_ISUID and S_ISGID) to temporarily represent
>>    SYSCTL_FLAG_MIN and SYSCTL_FLAG_MAX.
>> - added some helper macros.
>>
>> By introducing long min/max to replace void * extra1/extra2 in most cases,
>> unnecessary variables can be removed to save memory and avoid attacks.
>>
>> Signed-off-by: Wen Yang <wen.yang@linux.dev>
>> 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: Dave Young <dyoung@redhat.com>
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>   fs/proc/proc_sysctl.c  |  8 +++--
>>   include/linux/sysctl.h | 71 ++++++++++++++++++++++++++++++++++++------
>>   2 files changed, 67 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
>> index 90c99eb1abf6..e88d1dca2a80 100644
>> --- a/fs/proc/proc_sysctl.c
>> +++ b/fs/proc/proc_sysctl.c
>> @@ -848,8 +848,11 @@ static int proc_sys_getattr(struct mnt_idmap *idmap,
>>   		return PTR_ERR(head);
>>   
>>   	generic_fillattr(&nop_mnt_idmap, request_mask, inode, stat);
>> -	if (table)
>> +	if (table) {
>>   		stat->mode = (stat->mode & S_IFMT) | table->mode;
>> +		stat->mode &= ~SYSCTL_FLAG_MIN;
>> +		stat->mode &= ~SYSCTL_FLAG_MAX;
>> +	}
>>   
>>   	sysctl_head_finish(head);
>>   	return 0;
>> @@ -1163,7 +1166,8 @@ static int sysctl_check_table(const char *path, struct ctl_table_header *header)
>>   		if (!entry->proc_handler)
>>   			err |= sysctl_err(path, entry, "No proc_handler");
>>   
>> -		if ((entry->mode & (S_IRUGO|S_IWUGO)) != entry->mode)
>> +		if ((entry->mode & (S_IRUGO|S_IWUGO|SYSCTL_FLAG_MIN|SYSCTL_FLAG_MAX))
>> +		    != entry->mode)
>>   			err |= sysctl_err(path, entry, "bogus .mode 0%o",
>>   				entry->mode);
>>   	}
>> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
>> index 20e3914ec53f..8e27e8350ca8 100644
>> --- a/include/linux/sysctl.h
>> +++ b/include/linux/sysctl.h
>> @@ -28,6 +28,7 @@
>>   #include <linux/rbtree.h>
>>   #include <linux/uidgid.h>
>>   #include <uapi/linux/sysctl.h>
>> +#include <uapi/linux/stat.h>
>>   
>>   /* For the /proc/sys support */
>>   struct completion;
>> @@ -61,6 +62,37 @@ extern const int sysctl_vals[];
>>   
>>   extern const unsigned long sysctl_long_vals[];
>>   
>> +#define SYSCTL_NUM_ZERO                         (0L)
>> +#define SYSCTL_NUM_ONE                          (1L)
>> +#define SYSCTL_NUM_TWO                          (2L)
>> +#define SYSCTL_NUM_THREE                        (3L)
>> +#define SYSCTL_NUM_FOUR                         (4L)
>> +#define SYSCTL_NUM_FIVE                         (5L)
>> +#define SYSCTL_NUM_SIX                          (6L)
>> +#define SYSCTL_NUM_SEVEN                        (7L)
>> +#define SYSCTL_NUM_EIGHT                        (8L)
>> +#define SYSCTL_NUM_NINE                         (9L)
>> +#define SYSCTL_NUM_TEN                          (10L)
>> +#define SYSCTL_NUM_SIXTEEN                      (16L)
>> +#define SYSCTL_NUM_THIRTY_ONE                   (31L)
>> +#define SYSCTL_NUM_NEG_THIRTY_ONE               (-31L)
>> +#define SYSCTL_NUM_ONE_HUNDRED                  (100L)
>> +#define SYSCTL_NUM_TWO_HUNDRED                  (200L)
>> +#define SYSCTL_NUM_S8_MAX                       (127L)
>> +#define SYSCTL_NUM_U8_MAX                       (255L)
>> +#define SYSCTL_NUM_FIVE_HUNDRED                 (500L)
>> +#define SYSCTL_NUM_ONE_THOUSAND                 (1000L)
>> +#define SYSCTL_NUM_THREE_THOUSAND               (3000L)
>> +#define SYSCTL_NUM_16K                          (16 * 1024L)
>> +#define SYSCTL_NUM_16M                          (16 * 1024 * 1024L)
>> +#define SYSCTL_NUM_SEC_PER_HOUR                 (60 * 60L)
>> +#define SYSCTL_NUM_U16_MAX                      (65535L)
>> +#define SYSCTL_NUM_SEC_PER_DAY                  (24 * 60 * 60L)
>> +#define SYSCTL_NUM_MS_PER_DAY                   (24 * 60 * 60 * 1000L)
>> +#define SYSCTL_NUM_INT_MAX                      (INT_MAX)
>> +#define SYSCTL_NUM_NEG_ONE                      (-1)
>> +#define SYSCTL_NUM_LONG_MAX                     (LONG_MAX)
> 
> Are all those constants really needed?
> In the past they were useful as they referenced the array of constants.
> But now people can just use the number directly, or even better a
> subsystem-specific constant.
> 

Okay, we will make the modifications according to your suggestions.

>> +
>>   typedef int proc_handler(const struct ctl_table *ctl, int write, void *buffer,
>>   		size_t *lenp, loff_t *ppos);
>>   
>> @@ -131,6 +163,9 @@ 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)
>>   
>> +#define  SYSCTL_FLAG_MIN			S_ISUID
>> +#define  SYSCTL_FLAG_MAX			S_ISGID
> 
> Is it ever useful to have only one of these flags?
> IMO this could be a single flag.
>

There are a few  edge cases that require two flags, such as the 
following code snippet:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/sctp/sysctl.c#n103

	[SCTP_RTO_MIN_IDX] = {
		.procname	= "rto_min",
		.data		= &init_net.sctp.rto_min,
...
		.extra1         = SYSCTL_ONE,
		.extra2         = &init_net.sctp.rto_max
	},

extra1 points to a constant unsigned integer, but extra2 points to a 
field in the struct sctp.association structure

>> +
>>   /* A sysctl table is an array of struct ctl_table: */
>>   struct ctl_table {
>>   	const char *procname;		/* Text ID for /proc/sys, or zero */
>> @@ -139,8 +174,16 @@ struct ctl_table {
>>   	umode_t mode;
>>   	proc_handler *proc_handler;	/* Callback for text formatting */
>>   	struct ctl_table_poll *poll;
>> -	void *extra1;
>> -	void *extra2;
>> +	union {
>> +		struct {
>> +			void *extra1;
>> +			void *extra2;
>> +		};
>> +		struct {
>> +			long min;
>> +			long max;
>> +		};
>> +	};
>>   } __randomize_layout;
>>   
>>   struct ctl_node {
>> @@ -214,42 +257,50 @@ struct ctl_table_root {
>>   
>>   static inline unsigned int sysctl_range_min_u8(const struct ctl_table *table)
>>   {
>> -	return (table->extra1) ? *(unsigned int *) table->extra1 : 0;
>> +	return (table->mode & SYSCTL_FLAG_MIN) ? table->min :
>> +	       (table->extra1) ? *(unsigned int *) table->extra1 : 0;
> 
> All these repetitive functions could probably be replaced by one macro.
> Keeping the users more direct and avoiding all the repetition.
> 

Okay, we will make the modifications according to your suggestions and 
send v4 soon.

--
Best wishes,
Wen