From: Christian Brauner <christian@brauner.io>
To: keescook@chromium.org, linux-kernel@vger.kernel.org
Cc: ebiederm@xmission.com, mcgrof@kernel.org,
akpm@linux-foundation.org, joe.lawrence@redhat.com,
longman@redhat.com, linux@dominikbrodowski.net,
viro@zeniv.linux.org.uk, Christian Brauner <christian@brauner.io>
Subject: [PATCH v1 1/2] sysctl: cap to ULONG_MAX in proc_get_long()
Date: Mon, 15 Oct 2018 12:55:43 +0200 [thread overview]
Message-ID: <20181015105544.4395-2-christian@brauner.io> (raw)
In-Reply-To: <20181015105544.4395-1-christian@brauner.io>
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
next prev parent reply other threads:[~2018-10-15 10:56 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-15 10:55 [PATCH v1 0/2] sysctl: cap file-max value at ULONG_MAX Christian Brauner
2018-10-15 10:55 ` Christian Brauner [this message]
2018-10-15 16:18 ` [PATCH v1 1/2] sysctl: cap to ULONG_MAX in proc_get_long() Kees Cook
2018-10-15 16:30 ` Christian Brauner
2018-10-15 19:01 ` Christian Brauner
2018-10-15 10:55 ` [PATCH v1 2/2] sysctl: handle overflow for file-max Christian Brauner
2018-10-15 16:11 ` Kees Cook
2018-10-15 16:28 ` Christian Brauner
2018-10-15 21:20 ` Kees Cook
2018-10-16 13:16 ` Christian Brauner
2018-10-16 14:38 ` Christian Brauner
2018-10-16 15:13 ` Waiman Long
2018-10-16 15:21 ` Christian Brauner
2018-10-16 15:25 ` Waiman Long
2018-10-16 15:29 ` Christian Brauner
2018-10-16 15:33 ` Christian Brauner
2018-10-16 15:34 ` Waiman Long
2018-10-16 15:40 ` Christian Brauner
2018-10-16 15:44 ` Waiman Long
2018-10-16 15:47 ` Christian Brauner
2018-10-16 15:53 ` Waiman Long
2018-10-16 15:59 ` Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181015105544.4395-2-christian@brauner.io \
--to=christian@brauner.io \
--cc=akpm@linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=joe.lawrence@redhat.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@dominikbrodowski.net \
--cc=longman@redhat.com \
--cc=mcgrof@kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®