| 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: <20250105152853.211037-1-wen.yang@linux.dev>
Date: Sun, 5 Jan 2025 23:28:53 +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: [PATCH v5] sysctl: simplify the min/max boundary check
do_proc_dointvec_conv() used the default range of int type, while
do_proc_dointvec_minmax_conv() additionally used "int * {min, max}" of
struct do_proc_dointvec_minmax_conv_param, which are actually passed
in table->extra{1,2} pointers.
By changing the struct do_proc_dointvec_minmax_conv_param's
"int * {min, max}" to "int {min, max}" and setting the min/max value
via dereference the table->extra{1, 2}, those do_proc_dointvec_conv() and
do_proc_dointvec_minmax_conv() functions can be merged into one and reduce
code by one-third.
Similar changes were also made for do_proc_douintvec_minmax_conv_param,
do_proc_douintvec_conv() and do_proc_douintvec_minmax_conv().
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@...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 | 211 ++++++++++++++++++++----------------------------
1 file changed, 86 insertions(+), 125 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 7ae7a4136855..250dc9057259 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -424,22 +424,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_dointvec_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
+ * 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_dointvec_minmax_conv_param {
+ int min;
+ int max;
+};
+
+static inline int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
+ int *valp,
+ int write, void *data)
{
+ struct do_proc_dointvec_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;
@@ -448,21 +470,44 @@ 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)
+/**
+ * struct do_proc_douintvec_minmax_conv_param - proc_douintvec_minmax() range checking structure
+ * @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;
+};
+
+static inline int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
+ unsigned int *valp,
+ int write, void *data)
{
+ struct do_proc_douintvec_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;
}
@@ -489,7 +534,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))
@@ -667,7 +712,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,
@@ -762,7 +807,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);
}
/*
@@ -808,46 +853,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
@@ -867,53 +872,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_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
- *
- * 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
@@ -936,10 +902,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 +932,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 +983,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 +994,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 +1028,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 +1198,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 +1230,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