Messages in this thread Patch in this message | | | From | Christian Brauner <> | Subject | [PATCH v1 1/2] sysctl: cap to ULONG_MAX in proc_get_long() | Date | Mon, 15 Oct 2018 12:55:43 +0200 |
| |
proc_get_long() is a funny function. It uses simple_strtoul() and for a good reason. proc_get_long() wants to always succeed the parse and return the maybe incorrect value and the trailing characters to check against a pre-defined list of acceptable trailing values. However, simple_strtoul() explicitly ignores overflows which can cause funny things like the following to happen:
echo 18446744073709551616 > /proc/sys/fs/file-max cat /proc/sys/fs/file-max 0
(Which will cause your system to silently die behind your back.)
On the other hand kstrtoul() does do overflow detection but fails the parse in this case, does not return the trailing characters, and also fails the parse when anything other than '\n' is a trailing character whereas proc_get_long() wants to be more lenient.
Now, before adding another kstrtoul() function let's simply add a static parse strtoul_cap_erange() which does: - returns ULONG_MAX on ERANGE - returns the trailing characters to the caller This guarantees that we don't regress userspace in any way but also caps any parsed value to ULONG_MAX and prevents things like file-max to become 0 on overflow.
Cc: Kees Cook <keescook@chromium.org> Signed-off-by: Christian Brauner <christian@brauner.io> --- v0->v1: - s/sysctl_strtoul_lenient/strtoul_cap_erange/g - (Al) remove bool overflow return argument from strtoul_cap_erange - (Al) return ULONG_MAX on ERANGE from strtoul_cap_erange - (Dominik) fix spelling in commit message --- kernel/sysctl.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c index cc02050fd0c4..97551eb42946 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -67,6 +67,7 @@ #include <linux/bpf.h> #include <linux/mount.h> #include <linux/pipe_fs_i.h> +#include <../lib/kstrtox.h> #include <linux/uaccess.h> #include <asm/processor.h> @@ -2065,6 +2066,25 @@ static void proc_skip_char(char **buf, size_t *size, const char v) } } +static unsigned long strtoul_cap_erange(const char *cp, char **endp, + unsigned int base) +{ + unsigned long long result; + unsigned int rv; + + cp = _parse_integer_fixup_radix(cp, &base); + rv = _parse_integer(cp, base, &result); + if ((rv & KSTRTOX_OVERFLOW) || (result != (unsigned long)result)) + result = ULONG_MAX; + + cp += (rv & ~KSTRTOX_OVERFLOW); + + if (endp) + *endp = (char *)cp; + + return result; +} + #define TMPBUFLEN 22 /** * proc_get_long - reads an ASCII formatted integer from a user buffer @@ -2108,7 +2128,7 @@ static int proc_get_long(char **buf, size_t *size, if (!isdigit(*p)) return -EINVAL; - *val = simple_strtoul(p, &p, 0); + *val = strtoul_cap_erange(p, &p, 0); len = p - tmp; -- 2.17.1
| |