1. Patchew
  2. linux
  3. View series

[PATCH v2] sysctl: simplify the min/max boundary check

Wen Yang posted 1 patch 1 year, 6 months ago
There is a newer version of this series
kernel/sysctl.c | 107 ++++++++++++++++++++++++++----------------------
1 file changed, 57 insertions(+), 50 deletions(-)
[PATCH v2] sysctl: simplify the min/max boundary check
Posted by Wen Yang 1 year, 6 months ago 
The do_proc_dointvec_minmax_conv_param structure provides the minimum and
maximum values for doing range checking for the proc_dointvec_minmax()
handler, while the do_proc_douintvec_minmax_conv_param structure also
provides the minimum and maximum values for doing range checking for the
proc_douintvec_minmax()/proc_dou8vec_minmax() handlers.

To avoid duplicate code, a new do_proc_minmax_conv_param structure has been
introduced to replace both do_proc_dointvec_minmax_conv_param and
do_proc_douintvec_minmax_conv_param mentioned above.

This also prepares for the removal of sysctl_vals and sysctl_long_vals.

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 | 107 ++++++++++++++++++++++++++----------------------
 1 file changed, 57 insertions(+), 50 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index e4421594fc25..bc784d726e09 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -809,17 +809,18 @@ static int proc_taint(struct ctl_table *table, int write,
 }
 
 /**
- * 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
+ * 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_dointvec_minmax_conv_param structure provides the
+ * 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() handler.
+ * parameters that use the proc_dointvec_minmax(), proc_douintvec_minmax(),
+ * proc_dou8vec_minmax() and so on.
  */
-struct do_proc_dointvec_minmax_conv_param {
-	int *min;
-	int *max;
+struct do_proc_minmax_conv_param {
+	long min;
+	long max;
 };
 
 static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
@@ -827,7 +828,7 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
 					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.
@@ -839,8 +840,7 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
 		return ret;
 
 	if (write) {
-		if ((param->min && *param->min > tmp) ||
-		    (param->max && *param->max < tmp))
+		if ((param->min > tmp) || (param->max < tmp))
 			return -EINVAL;
 		WRITE_ONCE(*valp, tmp);
 	}
@@ -867,35 +867,27 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
 int proc_dointvec_minmax(struct ctl_table *table, int write,
 		  void *buffer, size_t *lenp, loff_t *ppos)
 {
-	struct do_proc_dointvec_minmax_conv_param param = {
-		.min = (int *) table->extra1,
-		.max = (int *) table->extra2,
+	struct do_proc_minmax_conv_param param = {
+		.min = INT_MIN,
+		.max = INT_MAX,
 	};
+
+	if (table->extra1)
+		param.min = *(int *) table->extra1;
+	if (table->extra2)
+		param.max = *(int *) table->extra2;
+
 	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;
+	struct do_proc_minmax_conv_param *param = data;
 	/* write via temporary local uint for bounds-checking */
 	unsigned int *up = write ? &tmp : valp;
 
@@ -904,8 +896,7 @@ static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
 		return ret;
 
 	if (write) {
-		if ((param->min && *param->min > tmp) ||
-		    (param->max && *param->max < tmp))
+		if ((param->min > tmp) || (param->max < tmp))
 			return -ERANGE;
 
 		WRITE_ONCE(*valp, tmp);
@@ -936,10 +927,16 @@ static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
 int proc_douintvec_minmax(struct ctl_table *table, int write,
 			  void *buffer, size_t *lenp, loff_t *ppos)
 {
-	struct do_proc_douintvec_minmax_conv_param param = {
-		.min = (unsigned int *) table->extra1,
-		.max = (unsigned int *) table->extra2,
+	struct do_proc_minmax_conv_param param = {
+		.min = 0,
+		.max = UINT_MAX,
 	};
+
+	if (table->extra1)
+		param.min = *(unsigned int *) table->extra1;
+	if (table->extra2)
+		param.max = *(unsigned int *) table->extra2;
+
 	return do_proc_douintvec(table, write, buffer, lenp, ppos,
 				 do_proc_douintvec_minmax_conv, &param);
 }
@@ -965,11 +962,11 @@ int proc_dou8vec_minmax(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 = {
+		.min = 0,
+		.max = 255U,
 	};
 	int res;
 
@@ -978,9 +975,9 @@ int proc_dou8vec_minmax(struct ctl_table *table, int write,
 		return -EINVAL;
 
 	if (table->extra1)
-		min = *(unsigned int *) table->extra1;
+		param.min = *(unsigned int *) table->extra1;
 	if (table->extra2)
-		max = *(unsigned int *) table->extra2;
+		param.max = *(unsigned int *) table->extra2;
 
 	tmp = *table;
 
@@ -1022,7 +1019,8 @@ 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 min = 0, max = ULONG_MAX;
+	unsigned long *i;
 	int vleft, first = 1, err = 0;
 	size_t left;
 	char *p;
@@ -1033,8 +1031,12 @@ static int __do_proc_doulongvec_minmax(void *data,
 	}
 
 	i = data;
-	min = table->extra1;
-	max = table->extra2;
+
+	if (table->extra1)
+		min = *(unsigned long *) table->extra1;
+	if (table->extra2)
+		max = *(unsigned long *) table->extra2;
+
 	vleft = table->maxlen / sizeof(unsigned long);
 	left = *lenp;
 
@@ -1066,7 +1068,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;
 			}
@@ -1224,7 +1226,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.
@@ -1236,8 +1238,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;
 	}
@@ -1269,10 +1270,16 @@ int proc_dointvec_jiffies(struct ctl_table *table, int write,
 int proc_dointvec_ms_jiffies_minmax(struct ctl_table *table, int write,
 			  void *buffer, size_t *lenp, loff_t *ppos)
 {
-	struct do_proc_dointvec_minmax_conv_param param = {
-		.min = (int *) table->extra1,
-		.max = (int *) table->extra2,
+	struct do_proc_minmax_conv_param param = {
+		.min = INT_MIN,
+		.max = INT_MAX,
 	};
+
+	if (table->extra1)
+		param.min = *(int *) table->extra1;
+	if (table->extra2)
+		param.max = *(int *) table->extra2;
+
 	return do_proc_dointvec(table, write, buffer, lenp, ppos,
 			do_proc_dointvec_ms_jiffies_minmax_conv, &param);
 }
-- 
2.25.1
Re: [PATCH v2] sysctl: simplify the min/max boundary check
Posted by Joel Granados 1 year, 6 months ago 
On Mon, Jul 15, 2024 at 11:02:23PM +0800, Wen Yang wrote:
> The do_proc_dointvec_minmax_conv_param structure provides the minimum and
> maximum values for doing range checking for the proc_dointvec_minmax()
> handler, while the do_proc_douintvec_minmax_conv_param structure also
> provides the minimum and maximum values for doing range checking for the
> proc_douintvec_minmax()/proc_dou8vec_minmax() handlers.
> 
> To avoid duplicate code, a new do_proc_minmax_conv_param structure has been
> introduced to replace both do_proc_dointvec_minmax_conv_param and
> do_proc_douintvec_minmax_conv_param mentioned above.
> 
> This also prepares for the removal of sysctl_vals and sysctl_long_vals.
> 
> 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 | 107 ++++++++++++++++++++++++++----------------------
This does not cleanly apply to current 6.11-rc1. Please send a v3 that
applies to current release. Thx

Best

-- 

Joel Granados
Re: [PATCH v2] sysctl: simplify the min/max boundary check
Posted by Wen Yang 1 year, 6 months ago 

On 2024/7/31 15:19, Joel Granados wrote:
> On Mon, Jul 15, 2024 at 11:02:23PM +0800, Wen Yang wrote:
>> The do_proc_dointvec_minmax_conv_param structure provides the minimum and
>> maximum values for doing range checking for the proc_dointvec_minmax()
>> handler, while the do_proc_douintvec_minmax_conv_param structure also
>> provides the minimum and maximum values for doing range checking for the
>> proc_douintvec_minmax()/proc_dou8vec_minmax() handlers.
>>
>> To avoid duplicate code, a new do_proc_minmax_conv_param structure has been
>> introduced to replace both do_proc_dointvec_minmax_conv_param and
>> do_proc_douintvec_minmax_conv_param mentioned above.
>>
>> This also prepares for the removal of sysctl_vals and sysctl_long_vals.
>>
>> 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 | 107 ++++++++++++++++++++++++++----------------------
> This does not cleanly apply to current 6.11-rc1. Please send a v3 that
> applies to current release. Thx

Thanks. I will do the change and send v3 soon.

--
Best wishes,
Wen

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.