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

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

[Wen Yang]

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 these min/max values for doing range checking for the
proc_douintvec_minmax()/proc_dou8vec_minmax() handlers.

To avoid duplicate code, a new 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 | 93 +++++++++++++++++++------------------------------
 1 file changed, 36 insertions(+), 57 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 79e6cb1d5c48..92305cdbb94a 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -809,17 +809,18 @@ static int proc_taint(const 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 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 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 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 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,21 @@ 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 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;
+	struct proc_minmax_conv_param *param = data;
 	/* write via temporary local uint for bounds-checking */
 	unsigned int *up = write ? &tmp : valp;
 
@@ -904,8 +890,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 +921,10 @@ 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 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);
 }
@@ -965,23 +950,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 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);
@@ -1022,7 +1001,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;
@@ -1033,8 +1012,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;
 
@@ -1066,7 +1046,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 +1204,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 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 +1216,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 +1248,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 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

Re: [RESEND PATCH v3] sysctl: simplify the min/max boundary check

[Joel Granados]

On Thu, Sep 05, 2024 at 09:48:18PM +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 these min/max values for doing range checking for the
> proc_douintvec_minmax()/proc_dou8vec_minmax() handlers.
> 
> To avoid duplicate code, a new 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
Thx for the remind. Its a bit to late to include this for 6.12. Will put it in
sysctl-testing setting it up for 6.13 (or whatever the next version will be).

Best

-- 

Joel Granados

Re: [RESEND PATCH v3] sysctl: simplify the min/max boundary check

[Joel Granados]

On Thu, Sep 05, 2024 at 09:48:18PM +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 these min/max values for doing range checking for the
> proc_douintvec_minmax()/proc_dou8vec_minmax() handlers.
Finally got around to reviewing this. Sorry for the wait. Thx for the
patch but I don't like how this looks in terms of Integer promotion in
32b architectures.

> 
> To avoid duplicate code, a new proc_minmax_conv_param structure has been
I'm not seeing duplicate code here as one is handling the int case and
the other is handling the uint case. And it is making sure that all
assignments and comparisons are without any Integer Promotion issues.
I'm not saying that it cannot be done, but it has to address Integer
Promotion issues in 32b architectures. 

> 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.
If I'm not mistaken this is another patchset that you sent separetly. Is
it "sysctl: encode the min/max values directly in the table entry"?

...

> @@ -904,8 +890,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 +921,10 @@ 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 proc_minmax_conv_param param;
> +
> +	param.min = (table->extra1) ? *(unsigned int *) table->extra1 : 0;
> +	param.max = (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
This is one of the cases where there is potential issues. Here, if the
unsigned integer value of table->extra{1,2}'s value is greater than when
the maximum value of a signed long, then the value assigned would be
incorrect. Note that the problem does not go away if you remove the
"unsigned" qualifier; it remains if table->extra{1,2} are originally
unsigned.

I'm not sure if there are more, but just having one of these things
around make me uncomfortable. Please re-work the patch in order to
remove this issue in order to continue review.

best

-- 

Joel Granados

Re: [RESEND PATCH v3] sysctl: simplify the min/max boundary check

[Wen Yang]

On 2024/10/23 03:12, Joel Granados wrote:
> On Thu, Sep 05, 2024 at 09:48:18PM +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 these min/max values for doing range checking for the
>> proc_douintvec_minmax()/proc_dou8vec_minmax() handlers.
> Finally got around to reviewing this. Sorry for the wait. Thx for the
> patch but I don't like how this looks in terms of Integer promotion in
> 32b architectures.
> 
Yes, thank you for pointing it out.
We will explain it later.

>>
>> To avoid duplicate code, a new proc_minmax_conv_param structure has been
> I'm not seeing duplicate code here as one is handling the int case and
> the other is handling the uint case. And it is making sure that all
> assignments and comparisons are without any Integer Promotion issues.
> I'm not saying that it cannot be done, but it has to address Integer
> Promotion issues in 32b architectures.
> 
>> 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.
> If I'm not mistaken this is another patchset that you sent separetly. Is
> it "sysctl: encode the min/max values directly in the table entry"?
> 

Yes.

> ...
> 
>> @@ -904,8 +890,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 +921,10 @@ 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 proc_minmax_conv_param param;
>> +
>> +	param.min = (table->extra1) ? *(unsigned int *) table->extra1 : 0;
>> +	param.max = (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
> This is one of the cases where there is potential issues. Here, if the
>  value of table->extra{1,2}'s value is greater than when
> the maximum value of a signed long, then the value assigned would be
> incorrect. Note that the problem does not go away if you remove the
> "unsigned" qualifier; it remains if table->extra{1,2} are originally
> unsigned.
> 

I set up a CentOS 7.9 32-bit VM on Virtuanbox:
# uname  -a
Linux osboxes.org 3.10.0-1160.2.2.el7.centos.plus.i686 #1 SMP Mon Oct 26 
11:56:29 UTC 2020 i686 i686 i386 GNU/Linux

And the following test code:

#include <stdio.h>
#include <stdlib.h>

int main()
{
	unsigned int i = 4294967294;
	long j = i;

	printf("original hex(i) = 0x%x\n", i);
	printf("unsigned int(i) = %lu\n", i);
	printf("---------------------\n");
	printf("hex(j) = 0x%x\n", j);
	printf("long(j) = %ld\n", j);
	printf("unsigned long(j) = %lu\n", j);
	printf("int(j) = %d\n", j);
	printf("unsigned int(j) = %lu\n", j);
	return 0;
}


./a.out

original hex(i) = 0xfffffffe
unsigned int(i) = 4294967294
---------------------
hex(j) = 0xfffffffe
long(j) = -2
unsigned long(j) = 4294967294
int(j) = -2
unsigned int(j) = 4294967294


The original hexadecimal values are the same, using unsigned int, int, 
unsigned long, or long is just interpreted in different ways.

We also ensure consistency in numerical writing and type conversion in 
the patch. For example, in proc_rointvec_jiffies, convert to int; And in 
proc_rouintvec_minmax, it is converted to unsigned int.


--
Best wishes,
Wen


> I'm not sure if there are more, but just having one of these things
> around make me uncomfortable. Please re-work the patch in order to
> remove this issue in order to continue review.
> 
> best
> 

Re: Re: [RESEND PATCH v3] sysctl: simplify the min/max boundary check

[Joel Granados]

On Wed, Oct 30, 2024 at 12:26:17AM +0800, Wen Yang wrote:
> 
> 
> On 2024/10/23 03:12, Joel Granados wrote:
> > On Thu, Sep 05, 2024 at 09:48:18PM +0800, Wen Yang wrote:
...

> >> @@ -936,10 +921,10 @@ 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 proc_minmax_conv_param param;
> >> +
> >> +	param.min = (table->extra1) ? *(unsigned int *) table->extra1 : 0;
> >> +	param.max = (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
> > This is one of the cases where there is potential issues. Here, if the
> >  value of table->extra{1,2}'s value is greater than when
> > the maximum value of a signed long, then the value assigned would be
> > incorrect. Note that the problem does not go away if you remove the
> > "unsigned" qualifier; it remains if table->extra{1,2} are originally
> > unsigned.
> > 
> 
> I set up a CentOS 7.9 32-bit VM on Virtuanbox:
> # uname  -a
> Linux osboxes.org 3.10.0-1160.2.2.el7.centos.plus.i686 #1 SMP Mon Oct 26 
> 11:56:29 UTC 2020 i686 i686 i386 GNU/Linux
> 
> And the following test code:
> 
> #include <stdio.h>
> #include <stdlib.h>
> 
> int main()
> {
> 	unsigned int i = 4294967294;
> 	long j = i;
> 
> 	printf("original hex(i) = 0x%x\n", i);
> 	printf("unsigned int(i) = %lu\n", i);
> 	printf("---------------------\n");
> 	printf("hex(j) = 0x%x\n", j);
> 	printf("long(j) = %ld\n", j);
> 	printf("unsigned long(j) = %lu\n", j);
> 	printf("int(j) = %d\n", j);
> 	printf("unsigned int(j) = %lu\n", j);
> 	return 0;
> }
> 
> 
> ./a.out
> 
> original hex(i) = 0xfffffffe
> unsigned int(i) = 4294967294
> ---------------------
> hex(j) = 0xfffffffe
> long(j) = -2
This ^^^^^ is exactly what I expected. Thx for the test!

When you transfer that to your patch, it means that for certain cases
[1] the value resulting from the interpretation of param.{min,max}
(signed long) is going to be different than the value resulting from the
interpretation of table-extra{1,2} (unsigned int).

Here is another way of thinking about it:
We are avoiding bugs where a developer thinks they are handling longs,
when in reality they are handling unsinged ints; The result of
subtracting 1 from (-2) is very different from subtracting 1 from
4294967294.

> unsigned long(j) = 4294967294
> int(j) = -2
> unsigned int(j) = 4294967294
> 
> 
> The original hexadecimal values are the same, using unsigned int, int, 
> unsigned long, or long is just interpreted in different ways.
Exactly. Hex remains the same but the interpretation changes. And it is
there where pain lies.

Please re-work the patch without merging everything into
do_proc_douintvec_minmax_conv_param

> 
> We also ensure consistency in numerical writing and type conversion in 
> the patch. For example, in proc_rointvec_jiffies, convert to int; And in 
> proc_rouintvec_minmax, it is converted to unsigned int.
> 
> 
> --
> Best wishes,
> Wen
> 
> 
> > I'm not sure if there are more, but just having one of these things
> > around make me uncomfortable. Please re-work the patch in order to
> > remove this issue in order to continue review.
> > 
> > best
> > 
Best

[1] This is one case that I can imagine
  1. On 32 bit architectures
  2. Where UINT_MAX > LONG_MAX
  3. The value being assigned is greater LONG_MAX

-- 

Joel Granados

Re: [RESEND PATCH v3] sysctl: simplify the min/max boundary check

[Wen Yang]

On 2024/10/31 17:39, Joel Granados wrote:
> On Wed, Oct 30, 2024 at 12:26:17AM +0800, Wen Yang wrote:
>>
>>
>> On 2024/10/23 03:12, Joel Granados wrote:
>>> On Thu, Sep 05, 2024 at 09:48:18PM +0800, Wen Yang wrote:
> ...
> 
>>>> @@ -936,10 +921,10 @@ 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 proc_minmax_conv_param param;
>>>> +
>>>> +	param.min = (table->extra1) ? *(unsigned int *) table->extra1 : 0;
>>>> +	param.max = (table->extra2) ? *(unsigned int *) table->extra2 : UINT_MAX;
>>> This is one of the cases where there is potential issues. Here, if the
>>>   value of table->extra{1,2}'s value is greater than when
>>> the maximum value of a signed long, then the value assigned would be
>>> incorrect. Note that the problem does not go away if you remove the
>>> "unsigned" qualifier; it remains if table->extra{1,2} are originally
>>> unsigned.
>>>
>>
>> I set up a CentOS 7.9 32-bit VM on Virtuanbox:
>> # uname  -a
>> Linux osboxes.org 3.10.0-1160.2.2.el7.centos.plus.i686 #1 SMP Mon Oct 26
>> 11:56:29 UTC 2020 i686 i686 i386 GNU/Linux
>>
>> And the following test code:
>>
>> #include <stdio.h>
>> #include <stdlib.h>
>>
>> int main()
>> {
>> 	unsigned int i = 4294967294;
>> 	long j = i;
>>
>> 	printf("original hex(i) = 0x%x\n", i);
>> 	printf("unsigned int(i) = %lu\n", i);
>> 	printf("---------------------\n");
>> 	printf("hex(j) = 0x%x\n", j);
>> 	printf("long(j) = %ld\n", j);
>> 	printf("unsigned long(j) = %lu\n", j);
>> 	printf("int(j) = %d\n", j);
>> 	printf("unsigned int(j) = %lu\n", j);
>> 	return 0;
>> }
>>
>>
>> ./a.out
>>
>> original hex(i) = 0xfffffffe
>> unsigned int(i) = 4294967294
>> ---------------------
>> hex(j) = 0xfffffffe
>> long(j) = -2
> This ^^^^^ is exactly what I expected. Thx for the test!
> 
> When you transfer that to your patch, it means that for certain cases
> [1] the value resulting from the interpretation of param.{min,max}
> (signed long) is going to be different than the value resulting from the
> interpretation of table-extra{1,2} (unsigned int).
> 
> Here is another way of thinking about it:
> We are avoiding bugs where a developer thinks they are handling longs,
> when in reality they are handling unsinged ints; The result of
> subtracting 1 from (-2) is very different from subtracting 1 from
> 4294967294.
> 
>> unsigned long(j) = 4294967294
>> int(j) = -2
>> unsigned int(j) = 4294967294
>>
>>
>> The original hexadecimal values are the same, using unsigned int, int,
>> unsigned long, or long is just interpreted in different ways.
> Exactly. Hex remains the same but the interpretation changes. And it is
> there where pain lies.
> 
> Please re-work the patch without merging everything into
> do_proc_douintvec_minmax_conv_param
> 

Thanks.
I will make the modifications according to your suggestions and send v4 
soon.

--
Best wishes,
Wen