* [PATCH v2] sysctl: simplify the min/max boundary check
@ 2024-07-15 15:02 ` Wen Yang
2024-07-31 7:19 ` Joel Granados
0 siblings, 1 reply; 3+ messages in thread
From: Wen Yang @ 2024-07-15 15:02 UTC (permalink / raw)
To: Eric W . Biederman, Luis Chamberlain, Kees Cook, Joel Granados,
Christian Brauner
Cc: Wen Yang, Dave Young, linux-kernel
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, ¶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;
+ 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, ¶m);
}
@@ -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, ¶m);
}
--
2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread