All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/5] sysctl: encode the min/max values directly in the table entry
@ 2025-02-09 15:58 Wen Yang
  2025-02-09 15:58 ` [PATCH v5 1/5] sysctl: simplify the min/max boundary check Wen Yang
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Wen Yang @ 2025-02-09 15:58 UTC (permalink / raw)
  To: Joel Granados, Luis Chamberlain, Kees Cook
  Cc: Eric W . Biederman, Dave Young, Christian Brauner,
	Thomas Weißschuh, linux-kernel, 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: 

- One of the things that happens and that is worth acknowledging is there
- is code that wraps proc_doulongvec_minmax and proc_dointvec_minmax.
- Having the minmax information separate from the data pointer makes that
- wrapping easier.

- Further the min/max information is typically separate from other kinds
- of data.  So even when not wrapped it is nice just to take a quick
- glance and see what the minimums and maximums are.

- My original suggest was that we change struct ctl_table from:

- > /* A sysctl table is an array of struct ctl_table: */
- > struct ctl_table {
- > 	const char *procname;		/* Text ID for /proc/sys */
- > 	void *data;
- > 	int maxlen;
- > 	umode_t mode;
- > 	proc_handler *proc_handler;	/* Callback for text formatting */
- > 	struct ctl_table_poll *poll;
- > 	void *extra1;
- > 	void *extra2;
- > } __randomize_layout;

- to:

- > /* A sysctl table is an array of struct ctl_table: */
- > struct ctl_table {
- > 	const char *procname;		/* Text ID for /proc/sys */
- > 	void *data;
- > 	int maxlen;
- > 	umode_t mode;
- > 	proc_handler *proc_handler;	/* Callback for text formatting */
- > 	struct ctl_table_poll *poll;
- >       unsigned long min;
- >       unsigned long max;
- > } __randomize_layout;

- That is just replace extra1 and extra2 with min and max members.  The
- members don't have any reason to be pointers.  Without being pointers
- the min/max functions can just use long values to cap either ints or
- longs, and there is no room for error.  The integer promotion rules
- will ensure that even negative values can be stored in unsigned long
- min and max values successfully.  Plus it is all bog standard C
- so there is nothing special to learn.

- There are a bunch of fiddly little details to transition from where we
- are today.  The most straightforward way I can see of making the
- transition is to add the min and max members.  Come up with replacements
- for proc_doulongvec_minmax and proc_dointvec_minmax that read the new
- min and max members.  Update all of the users.  Update the few users
- that use extra1 or extra2 for something besides min and max.  Then
- remove extra1 and extra2.  At the end it is simpler and requires the
- same or a little less space.

This patch series achieves direct encoding min/max values in table entries
and still maintains compatibility with existing extra1/extra2 pointers.
Afterwards, we can remove these unnecessary static variables progressively and
also finally kill the shared const array.

v4: https://lore.kernel.org/all/20241201140058.5653-1-wen.yang@linux.dev/
v3: https://lore.kernel.org/all/cover.1726365007.git.wen.yang@linux.dev/
v2: https://lore.kernel.org/all/tencent_143077FB953D8B549153BB07F54C5AA4870A@qq.com/
v1: https://lore.kernel.org/all/tencent_95D22FF919A42A99DA3C886B322CBD983905@qq.com/

and
https://lore.kernel.org/all/20250105152853.211037-1-wen.yang@linux.dev/

Wen Yang (5):
  sysctl: simplify the min/max boundary check
  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 the min/max encoding of sysctl
    table entries
  sysctl: delete six_hundred_forty_kb to save 4 bytes

 fs/proc/proc_sysctl.c  |  35 ++-
 include/linux/sysctl.h |  64 ++++-
 kernel/sysctl-test.c   | 581 +++++++++++++++++++++++++++++++++++++++++
 kernel/sysctl.c        | 211 ++++++---------
 4 files changed, 739 insertions(+), 152 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v5 1/5] sysctl: simplify the min/max boundary check
  2025-02-09 15:58 [PATCH v5 0/5] sysctl: encode the min/max values directly in the table entry Wen Yang
@ 2025-02-09 15:58 ` Wen Yang
  2025-02-09 15:58 ` [PATCH v5 2/5] sysctl: add helper functions to extract table->extra1/extra2 Wen Yang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Wen Yang @ 2025-02-09 15:58 UTC (permalink / raw)
  To: Joel Granados, Luis Chamberlain, Kees Cook
  Cc: Eric W . Biederman, Dave Young, Christian Brauner,
	Thomas Weißschuh, linux-kernel, Wen Yang, Joel Granados

Eric pointed out:
- That is just replace extra1 and extra2 with min and max members.  The
- members don't have any reason to be pointers.  Without being pointers
- the min/max functions can just use long values to cap either ints or
- longs, and there is no room for error.  The integer promotion rules
- will ensure that even negative values can be stored in unsigned long
- min and max values successfully.  Plus it is all bog standard C
- so there is nothing special to learn.

In order to archive it, we first introduce a new struct
do_proc_minmax_conv_param to replace both
do_proc_dointvec_minmax_conv_param and do_proc_douintvec_minmax_conv_param,
both int *{min, max} and unsigned int *{min, max} are changed to
unsigned long {min, max}, both do_proc_dointvec_conv() and
do_proc_douintvec_conv() deleted.

Selftest were also made:

[23:16:38] ================ sysctl_test (11 subtests) =================
[23:16:38] [PASSED] sysctl_test_api_dointvec_null_tbl_data
[23:16:38] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
[23:16:38] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
[23:16:38] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
[23:16:38] [PASSED] sysctl_test_dointvec_read_happy_single_positive
[23:16:38] [PASSED] sysctl_test_dointvec_read_happy_single_negative
[23:16:38] [PASSED] sysctl_test_dointvec_write_happy_single_positive
[23:16:38] [PASSED] sysctl_test_dointvec_write_happy_single_negative
[23:16:38] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
[23:16:38] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
[23:16:38] [PASSED] sysctl_test_register_sysctl_sz_invalid_extra_value
[23:16:38] =================== [PASSED] sysctl_test ===================
[23:16:38] ============================================================
[23:16:38] Testing complete. Ran 11 tests: passed: 11

Signed-off-by: Wen Yang <wen.yang@linux.dev>
Cc: Joel Granados <joel.granados@kernel.org>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
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 | 199 ++++++++++++++++++------------------------------
 1 file changed, 73 insertions(+), 126 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 21190098c21d..4507795e3568 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -409,22 +409,44 @@ static void proc_put_char(void **buf, size_t *size, char c)
 	}
 }
 
-static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp,
-				 int *valp,
-				 int write, void *data)
+/**
+ * struct do_proc_minmax_conv_param - proc_dointvec_minmax() range checking structure
+ * @min: the minimum allowable value
+ * @max: the maximum allowable value
+ *
+ * The do_proc_minmax_conv_param structure provides the
+ * minimum and maximum values for doing range checking for those sysctl
+ * parameters that use the proc_dointvec_minmax(), proc_dou8vec_minmax() and so on.
+ */
+struct do_proc_minmax_conv_param {
+	unsigned long min;
+	unsigned long max;
+};
+
+static inline int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
+		int *valp,
+		int write, void *data)
 {
+	struct do_proc_minmax_conv_param *param = data;
+	long min, max;
+	long val;
+
 	if (write) {
 		if (*negp) {
-			if (*lvalp > (unsigned long) INT_MAX + 1)
-				return -EINVAL;
-			WRITE_ONCE(*valp, -*lvalp);
+			max = param ? param->max : 0;
+			min = param ? param->min : INT_MIN;
+			val = -*lvalp;
 		} else {
-			if (*lvalp > (unsigned long) INT_MAX)
-				return -EINVAL;
-			WRITE_ONCE(*valp, *lvalp);
+			max = param ? param->max : INT_MAX;
+			min = param ? param->min : 0;
+			val = *lvalp;
 		}
+
+		if (val > max || val < min)
+			return -EINVAL;
+		WRITE_ONCE(*valp, (int)val);
 	} else {
-		int val = READ_ONCE(*valp);
+		val = (int)READ_ONCE(*valp);
 		if (val < 0) {
 			*negp = true;
 			*lvalp = -(unsigned long)val;
@@ -433,21 +455,30 @@ static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp,
 			*lvalp = (unsigned long)val;
 		}
 	}
+
 	return 0;
 }
 
-static int do_proc_douintvec_conv(unsigned long *lvalp,
-				  unsigned int *valp,
-				  int write, void *data)
+static inline int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
+					 unsigned int *valp,
+					 int write, void *data)
 {
+	struct do_proc_minmax_conv_param *param = data;
+	unsigned long min, max;
+
 	if (write) {
-		if (*lvalp > UINT_MAX)
+		max = param ? param->max : UINT_MAX;
+		min = param ? param->min : 0;
+
+		if (*lvalp > max || *lvalp < min)
 			return -EINVAL;
-		WRITE_ONCE(*valp, *lvalp);
+
+		WRITE_ONCE(*valp, (unsigned int)*lvalp);
 	} else {
 		unsigned int val = READ_ONCE(*valp);
 		*lvalp = (unsigned long)val;
 	}
+
 	return 0;
 }
 
@@ -474,7 +505,7 @@ static int __do_proc_dointvec(void *tbl_data, const struct ctl_table *table,
 	left = *lenp;
 
 	if (!conv)
-		conv = do_proc_dointvec_conv;
+		conv = do_proc_dointvec_minmax_conv;
 
 	if (write) {
 		if (proc_first_pos_non_zero_ignore(ppos, table))
@@ -652,7 +683,7 @@ static int __do_proc_douintvec(void *tbl_data, const struct ctl_table *table,
 	}
 
 	if (!conv)
-		conv = do_proc_douintvec_conv;
+		conv = do_proc_douintvec_minmax_conv;
 
 	if (write)
 		return do_proc_douintvec_w(i, table, buffer, lenp, ppos,
@@ -747,7 +778,7 @@ int proc_douintvec(const struct ctl_table *table, int write, void *buffer,
 		size_t *lenp, loff_t *ppos)
 {
 	return do_proc_douintvec(table, write, buffer, lenp, ppos,
-				 do_proc_douintvec_conv, NULL);
+				 do_proc_douintvec_minmax_conv, NULL);
 }
 
 /*
@@ -793,46 +824,6 @@ static int proc_taint(const struct ctl_table *table, int write,
 	return err;
 }
 
-/**
- * struct do_proc_dointvec_minmax_conv_param - proc_dointvec_minmax() range checking structure
- * @min: pointer to minimum allowable value
- * @max: pointer to maximum allowable value
- *
- * The do_proc_dointvec_minmax_conv_param structure provides the
- * minimum and maximum values for doing range checking for those sysctl
- * parameters that use the proc_dointvec_minmax() handler.
- */
-struct do_proc_dointvec_minmax_conv_param {
-	int *min;
-	int *max;
-};
-
-static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
-					int *valp,
-					int write, void *data)
-{
-	int tmp, ret;
-	struct do_proc_dointvec_minmax_conv_param *param = data;
-	/*
-	 * If writing, first do so via a temporary local int so we can
-	 * bounds-check it before touching *valp.
-	 */
-	int *ip = write ? &tmp : valp;
-
-	ret = do_proc_dointvec_conv(negp, lvalp, ip, write, data);
-	if (ret)
-		return ret;
-
-	if (write) {
-		if ((param->min && *param->min > tmp) ||
-		    (param->max && *param->max < tmp))
-			return -EINVAL;
-		WRITE_ONCE(*valp, tmp);
-	}
-
-	return 0;
-}
-
 /**
  * proc_dointvec_minmax - read a vector of integers with min/max values
  * @table: the sysctl table
@@ -852,53 +843,14 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
 int proc_dointvec_minmax(const 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_minmax_conv_param param;
+
+	param.min = (table->extra1) ? *(int *) table->extra1 : INT_MIN;
+	param.max = (table->extra2) ? *(int *) table->extra2 : INT_MAX;
 	return do_proc_dointvec(table, write, buffer, lenp, ppos,
 				do_proc_dointvec_minmax_conv, &param);
 }
 
-/**
- * struct do_proc_douintvec_minmax_conv_param - proc_douintvec_minmax() range checking structure
- * @min: pointer to minimum allowable value
- * @max: pointer to maximum allowable value
- *
- * The do_proc_douintvec_minmax_conv_param structure provides the
- * minimum and maximum values for doing range checking for those sysctl
- * parameters that use the proc_douintvec_minmax() handler.
- */
-struct do_proc_douintvec_minmax_conv_param {
-	unsigned int *min;
-	unsigned int *max;
-};
-
-static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
-					 unsigned int *valp,
-					 int write, void *data)
-{
-	int ret;
-	unsigned int tmp;
-	struct do_proc_douintvec_minmax_conv_param *param = data;
-	/* write via temporary local uint for bounds-checking */
-	unsigned int *up = write ? &tmp : valp;
-
-	ret = do_proc_douintvec_conv(lvalp, up, write, data);
-	if (ret)
-		return ret;
-
-	if (write) {
-		if ((param->min && *param->min > tmp) ||
-		    (param->max && *param->max < tmp))
-			return -ERANGE;
-
-		WRITE_ONCE(*valp, tmp);
-	}
-
-	return 0;
-}
-
 /**
  * proc_douintvec_minmax - read a vector of unsigned ints with min/max values
  * @table: the sysctl table
@@ -921,10 +873,11 @@ static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
 int proc_douintvec_minmax(const 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_minmax_conv_param param;
+
+	param.min = (table->extra1) ? *(unsigned int *) table->extra1 : 0;
+	param.max = (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
+
 	return do_proc_douintvec(table, write, buffer, lenp, ppos,
 				 do_proc_douintvec_minmax_conv, &param);
 }
@@ -950,23 +903,17 @@ int proc_dou8vec_minmax(const struct ctl_table *table, int write,
 			void *buffer, size_t *lenp, loff_t *ppos)
 {
 	struct ctl_table tmp;
-	unsigned int min = 0, max = 255U, val;
+	unsigned int val;
 	u8 *data = table->data;
-	struct do_proc_douintvec_minmax_conv_param param = {
-		.min = &min,
-		.max = &max,
-	};
+	struct do_proc_minmax_conv_param param;
 	int res;
 
 	/* Do not support arrays yet. */
 	if (table->maxlen != sizeof(u8))
 		return -EINVAL;
 
-	if (table->extra1)
-		min = *(unsigned int *) table->extra1;
-	if (table->extra2)
-		max = *(unsigned int *) table->extra2;
-
+	param.min = (table->extra1) ? *(unsigned int *) table->extra1 : 0;
+	param.max = (table->extra2) ? *(unsigned int *) table->extra2 : 255U;
 	tmp = *table;
 
 	tmp.maxlen = sizeof(val);
@@ -1007,7 +954,7 @@ static int __do_proc_doulongvec_minmax(void *data,
 		void *buffer, size_t *lenp, loff_t *ppos,
 		unsigned long convmul, unsigned long convdiv)
 {
-	unsigned long *i, *min, *max;
+	unsigned long *i, min, max;
 	int vleft, first = 1, err = 0;
 	size_t left;
 	char *p;
@@ -1018,8 +965,9 @@ static int __do_proc_doulongvec_minmax(void *data,
 	}
 
 	i = data;
-	min = table->extra1;
-	max = table->extra2;
+	min = (table->extra1) ? *(unsigned long *) table->extra1 : 0;
+	max = (table->extra2) ? *(unsigned long *) table->extra2 : ULONG_MAX;
+
 	vleft = table->maxlen / sizeof(unsigned long);
 	left = *lenp;
 
@@ -1051,7 +999,7 @@ static int __do_proc_doulongvec_minmax(void *data,
 			}
 
 			val = convmul * val / convdiv;
-			if ((min && val < *min) || (max && val > *max)) {
+			if ((val < min) || (val > max)) {
 				err = -EINVAL;
 				break;
 			}
@@ -1209,7 +1157,7 @@ static int do_proc_dointvec_ms_jiffies_minmax_conv(bool *negp, unsigned long *lv
 						int *valp, int write, void *data)
 {
 	int tmp, ret;
-	struct do_proc_dointvec_minmax_conv_param *param = data;
+	struct do_proc_minmax_conv_param *param = data;
 	/*
 	 * If writing, first do so via a temporary local int so we can
 	 * bounds-check it before touching *valp.
@@ -1221,8 +1169,7 @@ static int do_proc_dointvec_ms_jiffies_minmax_conv(bool *negp, unsigned long *lv
 		return ret;
 
 	if (write) {
-		if ((param->min && *param->min > tmp) ||
-				(param->max && *param->max < tmp))
+		if ((param->min > tmp) || (param->max < tmp))
 			return -EINVAL;
 		*valp = tmp;
 	}
@@ -1254,10 +1201,10 @@ int proc_dointvec_jiffies(const struct ctl_table *table, int write,
 int proc_dointvec_ms_jiffies_minmax(const 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_minmax_conv_param param;
+
+	param.min = (table->extra1) ? *(int *) table->extra1 : INT_MIN;
+	param.max = (table->extra2) ? *(int *) table->extra2 : INT_MAX;
 	return do_proc_dointvec(table, write, buffer, lenp, ppos,
 			do_proc_dointvec_ms_jiffies_minmax_conv, &param);
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v5 2/5] sysctl: add helper functions to extract table->extra1/extra2
  2025-02-09 15:58 [PATCH v5 0/5] sysctl: encode the min/max values directly in the table entry Wen Yang
  2025-02-09 15:58 ` [PATCH v5 1/5] sysctl: simplify the min/max boundary check Wen Yang
@ 2025-02-09 15:58 ` Wen Yang
  2025-02-09 15:58 ` [PATCH v5 3/5] sysctl: support encoding values directly in the table entry Wen Yang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Wen Yang @ 2025-02-09 15:58 UTC (permalink / raw)
  To: Joel Granados, Luis Chamberlain, Kees Cook
  Cc: Eric W . Biederman, Dave Young, Christian Brauner,
	Thomas Weißschuh, linux-kernel, 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 | 44 ++++++++++++++++++++++++++++++++++++++++++
 kernel/sysctl.c        | 21 ++++++++++----------
 3 files changed, 63 insertions(+), 23 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 27a283d85a6e..6649d1db5f8f 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 40a6ac6c9713..eee8480dc069 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -210,6 +210,50 @@ struct ctl_table_root {
 #define register_sysctl(path, table)	\
 	register_sysctl_sz(path, table, ARRAY_SIZE(table))
 
+#define __SYSCTL_RANGE_MIN(_a, _b, _c) (((_a)->extra1) ? *(_b((_a)->extra1)) : (_c))
+
+#define __SYSCTL_RANGE_MAX(_a, _b, _c) (((_a)->extra2) ? *(_b((_a)->extra2)) : (_c))
+
+static inline unsigned int sysctl_range_min_u8(const struct ctl_table *table)
+{
+	return (unsigned int)__SYSCTL_RANGE_MIN(table, (unsigned int *), 0);
+}
+
+static inline unsigned int sysctl_range_max_u8(const struct ctl_table *table)
+{
+	return (unsigned int)__SYSCTL_RANGE_MAX(table, (unsigned int *), U8_MAX);
+}
+
+static inline int sysctl_range_min_int(const struct ctl_table *table)
+{
+	return (int)__SYSCTL_RANGE_MIN(table, (int *), INT_MIN);
+}
+
+static inline int sysctl_range_max_int(const struct ctl_table *table)
+{
+	return (int)__SYSCTL_RANGE_MAX(table, (int *), INT_MAX);
+}
+
+static inline unsigned int sysctl_range_min_uint(const struct ctl_table *table)
+{
+	return (unsigned int)__SYSCTL_RANGE_MIN(table, (unsigned int *), 0);
+}
+
+static inline unsigned int sysctl_range_max_uint(const struct ctl_table *table)
+{
+	return (unsigned int)__SYSCTL_RANGE_MAX(table, (unsigned int *), UINT_MAX);
+}
+
+static inline unsigned long sysctl_range_min_ulong(const struct ctl_table *table)
+{
+	return (unsigned long)__SYSCTL_RANGE_MIN(table, (unsigned long *), 0);
+}
+
+static inline unsigned long sysctl_range_max_ulong(const struct ctl_table *table)
+{
+	return (unsigned long)__SYSCTL_RANGE_MAX(table, (unsigned long *), 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 4507795e3568..b745445cbfbf 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -845,8 +845,8 @@ int proc_dointvec_minmax(const struct ctl_table *table, int write,
 {
 	struct do_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);
 }
@@ -875,9 +875,8 @@ int proc_douintvec_minmax(const struct ctl_table *table, int write,
 {
 	struct do_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);
 }
@@ -912,8 +911,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);
@@ -965,8 +964,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;
@@ -1203,8 +1202,8 @@ int proc_dointvec_ms_jiffies_minmax(const struct ctl_table *table, int write,
 {
 	struct do_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


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v5 3/5] sysctl: support encoding values directly in the table entry
  2025-02-09 15:58 [PATCH v5 0/5] sysctl: encode the min/max values directly in the table entry Wen Yang
  2025-02-09 15:58 ` [PATCH v5 1/5] sysctl: simplify the min/max boundary check Wen Yang
  2025-02-09 15:58 ` [PATCH v5 2/5] sysctl: add helper functions to extract table->extra1/extra2 Wen Yang
@ 2025-02-09 15:58 ` Wen Yang
  2025-02-09 15:58 ` [PATCH v5 4/5] sysctl: add kunit test code to check the min/max encoding of sysctl table entries Wen Yang
  2025-02-09 15:58 ` [PATCH v5 5/5] sysctl: delete six_hundred_forty_kb to save 4 bytes Wen Yang
  4 siblings, 0 replies; 6+ messages in thread
From: Wen Yang @ 2025-02-09 15:58 UTC (permalink / raw)
  To: Joel Granados, Luis Chamberlain, Kees Cook
  Cc: Eric W . Biederman, Dave Young, Christian Brauner,
	Thomas Weißschuh, linux-kernel, 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 compatibility
  with previous code without increasing memory space, and then we could
  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, 2 - 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.

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  | 14 ++++++++++++--
 include/linux/sysctl.h | 24 ++++++++++++++++++++----
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 6649d1db5f8f..08b9cfb11165 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -848,9 +848,18 @@ 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;
 
+		/*
+		 * S_ISUID and S_ISGID are to temporarily represent
+		 * the internal SYSCTL_FLAG_MIN and SYSCTL_FLAG_MAX,
+		 * and are not visible to users
+		 */
+		stat->mode &= ~SYSCTL_FLAG_MIN;
+		stat->mode &= ~SYSCTL_FLAG_MAX;
+	}
+
 	sysctl_head_finish(head);
 	return 0;
 }
@@ -1163,7 +1172,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 eee8480dc069..f07a3f0fb3b1 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;
@@ -129,6 +130,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 */
@@ -137,8 +141,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 {
+			unsigned long min;
+			unsigned long max;
+		};
+	};
 } __randomize_layout;
 
 struct ctl_node {
@@ -210,9 +222,13 @@ struct ctl_table_root {
 #define register_sysctl(path, table)	\
 	register_sysctl_sz(path, table, ARRAY_SIZE(table))
 
-#define __SYSCTL_RANGE_MIN(_a, _b, _c) (((_a)->extra1) ? *(_b((_a)->extra1)) : (_c))
+#define __SYSCTL_RANGE_EXTRA1(_a, _b, _c) (((_a)->extra1) ? *(_b((_a)->extra1)) : (_c))
+#define __SYSCTL_RANGE_MIN(_a, _b, _c)    ((((_a)->mode) & SYSCTL_FLAG_MIN) ? \
+					   ((_a)->min) : __SYSCTL_RANGE_EXTRA1(_a, _b, _c))
 
-#define __SYSCTL_RANGE_MAX(_a, _b, _c) (((_a)->extra2) ? *(_b((_a)->extra2)) : (_c))
+#define __SYSCTL_RANGE_EXTRA2(_a, _b, _c) (((_a)->extra2) ? *(_b((_a)->extra2)) : (_c))
+#define __SYSCTL_RANGE_MAX(_a, _b, _c)    ((((_a)->mode) & SYSCTL_FLAG_MAX) ? \
+					   ((_a)->max) : __SYSCTL_RANGE_EXTRA2(_a, _b, _c))
 
 static inline unsigned int sysctl_range_min_u8(const struct ctl_table *table)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v5 4/5] sysctl: add kunit test code to check the min/max encoding of sysctl table entries
  2025-02-09 15:58 [PATCH v5 0/5] sysctl: encode the min/max values directly in the table entry Wen Yang
                   ` (2 preceding siblings ...)
  2025-02-09 15:58 ` [PATCH v5 3/5] sysctl: support encoding values directly in the table entry Wen Yang
@ 2025-02-09 15:58 ` Wen Yang
  2025-02-09 15:58 ` [PATCH v5 5/5] sysctl: delete six_hundred_forty_kb to save 4 bytes Wen Yang
  4 siblings, 0 replies; 6+ messages in thread
From: Wen Yang @ 2025-02-09 15:58 UTC (permalink / raw)
  To: Joel Granados, Luis Chamberlain, Kees Cook
  Cc: Eric W . Biederman, Dave Young, Christian Brauner,
	Thomas Weißschuh, linux-kernel, 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..47064ab8b7f3 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;
+	const 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		= -1,
+		.max		= 100,
+	};
+	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;
+	const struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(int),
+		.mode		= 0644 | SYSCTL_FLAG_MIN,
+		.proc_handler	= proc_dointvec_minmax,
+		.min		= -1,
+	};
+	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;
+	const struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(int),
+		.mode		= 0644 | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_dointvec_minmax,
+		.max		= 100,
+	};
+	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;
+	const 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		= 4,
+		.max		= 200,
+	};
+	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;
+	const struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(int),
+		.mode		= 0644 | SYSCTL_FLAG_MIN,
+		.proc_handler	= proc_douintvec_minmax,
+		.min		= 4,
+	};
+	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;
+	const struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(int),
+		.mode		= 0644 | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_douintvec_minmax,
+		.max		= 1000,
+	};
+	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;
+	const 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		= 3,
+		.max		= 100,
+	};
+	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;
+	const struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(unsigned char),
+		.mode		= 0644 | SYSCTL_FLAG_MIN,
+		.proc_handler	= proc_dou8vec_minmax,
+		.min		= 3,
+	};
+	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;
+	const struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(unsigned char),
+		.mode		= 0644 | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_dou8vec_minmax,
+		.max		= 200,
+	};
+	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;
+	const 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		= 1000,
+		.max		= 3000,
+	};
+	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;
+	const struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(unsigned long),
+		.mode		= 0644 | SYSCTL_FLAG_MIN,
+		.proc_handler	= proc_doulongvec_minmax,
+		.min		= 1000,
+	};
+	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;
+	const struct ctl_table table = {
+		.procname = "foo",
+		.data		= &data,
+		.maxlen		= sizeof(unsigned long),
+		.mode		= 0644 | SYSCTL_FLAG_MAX,
+		.proc_handler	= proc_doulongvec_minmax,
+		.max		= 3000,
+	};
+	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


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v5 5/5] sysctl: delete six_hundred_forty_kb to save 4 bytes
  2025-02-09 15:58 [PATCH v5 0/5] sysctl: encode the min/max values directly in the table entry Wen Yang
                   ` (3 preceding siblings ...)
  2025-02-09 15:58 ` [PATCH v5 4/5] sysctl: add kunit test code to check the min/max encoding of sysctl table entries Wen Yang
@ 2025-02-09 15:58 ` Wen Yang
  4 siblings, 0 replies; 6+ messages in thread
From: Wen Yang @ 2025-02-09 15:58 UTC (permalink / raw)
  To: Joel Granados, Luis Chamberlain, Kees Cook
  Cc: Eric W . Biederman, Dave Young, Christian Brauner,
	Thomas Weißschuh, linux-kernel, 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 b745445cbfbf..a9ef3b4d736c 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -81,13 +81,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;
 
@@ -1915,10 +1908,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		= 0,
+		.max		= 640 * 1024,
 	},
 	{
 		.procname	= "perf_event_max_contexts_per_stack",
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-09 16:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-09 15:58 [PATCH v5 0/5] sysctl: encode the min/max values directly in the table entry Wen Yang
2025-02-09 15:58 ` [PATCH v5 1/5] sysctl: simplify the min/max boundary check Wen Yang
2025-02-09 15:58 ` [PATCH v5 2/5] sysctl: add helper functions to extract table->extra1/extra2 Wen Yang
2025-02-09 15:58 ` [PATCH v5 3/5] sysctl: support encoding values directly in the table entry Wen Yang
2025-02-09 15:58 ` [PATCH v5 4/5] sysctl: add kunit test code to check the min/max encoding of sysctl table entries Wen Yang
2025-02-09 15:58 ` [PATCH v5 5/5] sysctl: delete six_hundred_forty_kb to save 4 bytes Wen Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.