From 8cf7630b29701d364f8df4a50e4f1f5e752b2778 Mon Sep 17 00:00:00 2001 From: Zev Weiss Date: Mon, 11 Mar 2019 23:28:02 -0700 Subject: kernel/sysctl.c: add missing range check in do_proc_dointvec_minmax_conv This bug has apparently existed since the introduction of this function in the pre-git era (4500e91754d3 in Thomas Gleixner's history.git, "[NET]: Add proc_dointvec_userhz_jiffies, use it for proper handling of neighbour sysctls."). As a minimal fix we can simply duplicate the corresponding check in do_proc_dointvec_conv(). Link: http://lkml.kernel.org/r/20190207123426.9202-3-zev@bewilderbeest.net Signed-off-by: Zev Weiss Cc: Brendan Higgins Cc: Iurii Zaikin Cc: Kees Cook Cc: Luis Chamberlain Cc: [2.6.2+] Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/sysctl.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'kernel/sysctl.c') diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 7bb3988425ee..0854197e0e67 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2645,7 +2645,16 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp, { struct do_proc_dointvec_minmax_conv_param *param = data; if (write) { - int val = *negp ? -*lvalp : *lvalp; + int val; + if (*negp) { + if (*lvalp > (unsigned long) INT_MAX + 1) + return -EINVAL; + val = -*lvalp; + } else { + if (*lvalp > (unsigned long) INT_MAX) + return -EINVAL; + val = *lvalp; + } if ((param->min && *param->min > val) || (param->max && *param->max < val)) return -EINVAL; -- cgit v1.2.3 From 2bc4fc60fb3e1a4cae429b889720755ebe9085bb Mon Sep 17 00:00:00 2001 From: Zev Weiss Date: Mon, 11 Mar 2019 23:28:06 -0700 Subject: kernel/sysctl.c: define minmax conv functions in terms of non-minmax versions do_proc_do[u]intvec_minmax_conv() had included open-coded versions of do_proc_do[u]intvec_conv(); the duplication led to buggy inconsistencies (missing range checks). To reduce the likelihood of such problems in the future, we can instead refactor both to be defined in terms of their non-bounded counterparts (plus the added check). Link: http://lkml.kernel.org/r/20190207165138.5oud57vq4ozwb4kh@hatter.bewilderbeest.net Signed-off-by: Zev Weiss Cc: Brendan Higgins Cc: Iurii Zaikin Cc: Kees Cook Cc: Luis Chamberlain Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/sysctl.c | 59 +++++++++++++++++++++++++-------------------------------- 1 file changed, 26 insertions(+), 33 deletions(-) (limited to 'kernel/sysctl.c') diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 0854197e0e67..e5da394d1ca3 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2643,32 +2643,25 @@ 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) { - int val; - if (*negp) { - if (*lvalp > (unsigned long) INT_MAX + 1) - return -EINVAL; - val = -*lvalp; - } else { - if (*lvalp > (unsigned long) INT_MAX) - return -EINVAL; - val = *lvalp; - } - if ((param->min && *param->min > val) || - (param->max && *param->max < val)) + if ((param->min && *param->min > tmp) || + (param->max && *param->max < tmp)) return -EINVAL; - *valp = val; - } else { - int val = *valp; - if (val < 0) { - *negp = true; - *lvalp = -(unsigned long)val; - } else { - *negp = false; - *lvalp = (unsigned long)val; - } + *valp = tmp; } + return 0; } @@ -2717,22 +2710,22 @@ 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; - if (write) { - unsigned int val = *lvalp; + ret = do_proc_douintvec_conv(lvalp, up, write, data); + if (ret) + return ret; - if (*lvalp > UINT_MAX) - return -EINVAL; - - if ((param->min && *param->min > val) || - (param->max && *param->max < val)) + if (write) { + if ((param->min && *param->min > tmp) || + (param->max && *param->max < tmp)) return -ERANGE; - *valp = val; - } else { - unsigned int val = *valp; - *lvalp = (unsigned long) val; + *valp = tmp; } return 0; -- cgit v1.2.3