| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20241201140058.5653-1-wen.yang@linux.dev>
Date: Sun, 1 Dec 2024 22:00:58 +0800
From: Wen Yang <wen.yang@...ux.dev>
To: Joel Granados <joel.granados@...nel.org>,
"Eric W . Biederman" <ebiederm@...ssion.com>,
Luis Chamberlain <mcgrof@...nel.org>,
Kees Cook <keescook@...omium.org>
Cc: Christian Brauner <brauner@...nel.org>,
Wen Yang <wen.yang@...ux.dev>,
Dave Young <dyoung@...hat.com>,
linux-kernel@...r.kernel.org
Subject: [RESEND PATCH v4] sysctl: simplify the min/max boundary check
The do_proc_dointvec_minmax_conv_param structure provides the minimum and
maximum values for doing range checking for the proc_dointvec_minmax()
handler, however min/max is a pointer and may be NULL, so the following
code snippet appears multiple times:
if ((param->min && *param->min > tmp) ||
(param->max && *param->max < tmp))
And int types also have min/max values, so when the pointer is NULL,
explicitly setting min/max to INT_{MIN/MAX} could simplify the code a bit.
Similar changes were also made for do_proc_douintvec_minmax_conv_param.
Signed-off-by: Wen Yang <wen.yang@...ux.dev>
Cc: Joel Granados <joel.granados@...nel.org>
Cc: Luis Chamberlain <mcgrof@...nel.org>
Cc: Kees Cook <keescook@...omium.org>
Cc: Eric W. Biederman <ebiederm@...ssion.com>
Cc: Christian Brauner <brauner@...nel.org>
Cc: Dave Young <dyoung@...hat.com>
Cc: linux-kernel@...r.kernel.org
---
kernel/sysctl.c | 75 ++++++++++++++++++++++---------------------------
1 file changed, 34 insertions(+), 41 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 79e6cb1d5c48..47e2fe4fe978 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -810,16 +810,16 @@ 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
+ * @min: the minimum allowable value
+ * @max: the 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.
+ * parameters that use the proc_dointvec_minmax(), proc_dou8vec_minmax() and so on.
*/
struct do_proc_dointvec_minmax_conv_param {
- int *min;
- int *max;
+ int min;
+ int max;
};
static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
@@ -839,8 +839,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,26 +866,26 @@ 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_dointvec_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, ¶m);
}
/**
* 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
+ * @min: minimum allowable value
+ * @max: 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;
+ unsigned int min;
+ unsigned int max;
};
static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
@@ -904,8 +903,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 +934,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_douintvec_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, ¶m);
}
@@ -965,23 +964,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_douintvec_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 +1015,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 +1026,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 +1060,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;
}
@@ -1236,8 +1230,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 +1262,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_dointvec_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, ¶m);
}
--
2.25.1
Powered by blists - more mailing lists