* binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying @ 2017-03-07 14:44 Leonard den Ottolander 2017-03-08 17:54 ` Carlos O'Donell 0 siblings, 1 reply; 17+ messages in thread From: Leonard den Ottolander @ 2017-03-07 14:44 UTC (permalink / raw) To: linux-api-u79uwXL29TY76Z2rM5mHXA In the article https://googleprojectzero.blogspot.nl/2014/08/the-poisoned-nul-byte-2014-edition.html the author describes launching an attack on an off by one NUL byte bug in glibc. He explains that both the memory leak in the option parsing of PolKits pkexec.c and the excessive value of MAX_ARG_STRINGS in the kernels include/uapi/linux/binfmts.h (#define MAX_ARG_STRINGS 0x7FFFFFFF) enabled his attack. The rationale for that excessive value is a bit odd: "MAX_ARG_STRINGS is chosen to fit in a signed 32-bit integer." https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/binfmts.h?id=607ca46e97a1b6594b29647d98a32d545c24bdff The fact that the integer is so large makes that on a 32 bit platform the entire heap can be initialized with values the attacker provides, given the memory leak in the option parsing of pkexec.c, an approach the author calls "heap spraying". (Note that two similar memory leaks exist in the option parsing of pkexecs sibling pkcheck.c, so pkcheck will also allow an attacker to heap spray its entire memory on a 32 bit system.) If you compare http://www.linuxjournal.com/article/6060 you will see that the TOTAL amount of memory reserved for command line arguments used to be 32 pages, i.e. 128KiB (#define MAX_ARG_PAGES 32). The amount of memory reserved for command line arguments in more current kernels is the multiplication of MAX_ARG_STRINGS and MAX_ARG_STRLEN. Both the memory leaks in pkexec.c and pkcheck.c seem very severe, but their impact would be much less if MAX_ARG_STRINGS would hold a sensible value. After some experimentation with $ rpmbuild -ba kernel.spec on CentOS-7 I've come up with values that allow this kernel compilation. As this kind of build is probably one of the most demanding actions in relation to MAX_ARG_STRINGS and MAX_ARG_STRLEN I believe below values to be sensible and much safer defaults: #define MAX_ARG_STRLEN 65536 #define MAX_ARG_STRINGS 4096 allow me to build a kernel on a system using those values. Since the above value of MAX_ARG_STRLEN is half of the current value we might leave it alone and only alter MAX_ARG_STRINGS, resulting in --- a/include/uapi/linux/binfmts.h 2016-11-23 21:02:31.000000000 +0100 +++ b/include/uapi/linux/binfmts.h 2017-02-07 15:40:13.548403615 +0100 @@ -12,7 +12,7 @@ struct pt_regs; * MAX_ARG_STRINGS is chosen to fit in a signed 32-bit integer. */ #define MAX_ARG_STRLEN (PAGE_SIZE * 32) -#define MAX_ARG_STRINGS 0x7FFFFFFF +#define MAX_ARG_STRINGS 4096 /* sizeof(linux_binprm->buf) */ #define BINPRM_BUF_SIZE 128 Filed a bug at https://bugzilla.kernel.org/show_bug.cgi?id=194809 . Regards, Leonard. -- mount -t life -o ro /dev/dna /genetic/research ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying 2017-03-07 14:44 binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying Leonard den Ottolander @ 2017-03-08 17:54 ` Carlos O'Donell [not found] ` <f7f9f60b-0f39-7dce-1778-3aa40ba198ef-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Carlos O'Donell @ 2017-03-08 17:54 UTC (permalink / raw) To: Leonard den Ottolander, linux-api-u79uwXL29TY76Z2rM5mHXA On 03/07/2017 09:44 AM, Leonard den Ottolander wrote: > After some experimentation with > $ rpmbuild -ba kernel.spec > on CentOS-7 I've come up with values that allow this kernel compilation. > As this kind of build is probably one of the most demanding actions in > relation to MAX_ARG_STRINGS and MAX_ARG_STRLEN I believe below values to > be sensible and much safer defaults: The most demanding application I've ever seen when playing with this is the compiler because it has to pass some very large strings from the driver to a subprocess. So much so that we have `@file` to use intermediate file storage to workaround kernel limits on various operating systems. > #define MAX_ARG_STRLEN 65536 > #define MAX_ARG_STRINGS 4096 > > allow me to build a kernel on a system using those values. > > Since the above value of MAX_ARG_STRLEN is half of the current value we > might leave it alone and only alter MAX_ARG_STRINGS, resulting in > > --- a/include/uapi/linux/binfmts.h 2016-11-23 21:02:31.000000000 +0100 > +++ b/include/uapi/linux/binfmts.h 2017-02-07 15:40:13.548403615 +0100 > @@ -12,7 +12,7 @@ struct pt_regs; > * MAX_ARG_STRINGS is chosen to fit in a signed 32-bit integer. > */ > #define MAX_ARG_STRLEN (PAGE_SIZE * 32) > -#define MAX_ARG_STRINGS 0x7FFFFFFF > +#define MAX_ARG_STRINGS 4096 In glibc we limit setuid applications, for example sanitizing their environment where it would cause problems or alter behaviour in unintended ways. Can we avoid imposing a limit on all applications? -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <f7f9f60b-0f39-7dce-1778-3aa40ba198ef-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying [not found] ` <f7f9f60b-0f39-7dce-1778-3aa40ba198ef-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2017-03-08 18:18 ` Leonard den Ottolander 2017-03-08 18:21 ` Leonard den Ottolander ` (2 more replies) 2017-03-08 18:48 ` Leonard den Ottolander 1 sibling, 3 replies; 17+ messages in thread From: Leonard den Ottolander @ 2017-03-08 18:18 UTC (permalink / raw) To: linux-api-u79uwXL29TY76Z2rM5mHXA On Wed, 2017-03-08 at 12:54 -0500, Carlos O'Donell wrote: > The most demanding application I've ever seen when playing with this is the > compiler because it has to pass some very large strings from the driver > to a subprocess. So much so that we have `@file` to use intermediate file > storage to workaround kernel limits on various operating systems. So a value that would satisfy glibc would suffice. Any problem with that limit of 4096 arguments? The size of each individual argument is still the old limit of 128KiB. By the way, my kernel build example seems to already catch your scenario. Kernel build uses a compiler. > In glibc we limit setuid applications, for example sanitizing their > environment where it would cause problems or alter behaviour in > unintended ways. > > Can we avoid imposing a limit on all applications? Not imposing a limit - btw, 0x7FFFFFFF *is* a limit albeit a ridiculous and dangerously large limit - is a bad idea, because it allows the aforementioned "heap-spraying" which is a serious attack vector. Note that 128KiB * 4096 arguments still adds up to 512MiB!!! If you don't feel the limit of 4096 arguments is sufficient please provide us with an example where that limit is insufficient. Regards, Leonard. -- mount -t life -o ro /dev/dna /genetic/research ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying 2017-03-08 18:18 ` Leonard den Ottolander @ 2017-03-08 18:21 ` Leonard den Ottolander 2017-03-08 20:47 ` Joseph Myers 2017-03-08 21:05 ` Carlos O'Donell 2 siblings, 0 replies; 17+ messages in thread From: Leonard den Ottolander @ 2017-03-08 18:21 UTC (permalink / raw) To: linux-api-u79uwXL29TY76Z2rM5mHXA On Wed, 2017-03-08 at 19:18 +0100, Leonard den Ottolander wrote: > So a value that would satisfy glibc would suffice. That of course should be "gcc" not "glibc". -- mount -t life -o ro /dev/dna /genetic/research ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying 2017-03-08 18:18 ` Leonard den Ottolander 2017-03-08 18:21 ` Leonard den Ottolander @ 2017-03-08 20:47 ` Joseph Myers 2017-03-08 21:05 ` Carlos O'Donell 2 siblings, 0 replies; 17+ messages in thread From: Joseph Myers @ 2017-03-08 20:47 UTC (permalink / raw) To: Leonard den Ottolander; +Cc: linux-api-u79uwXL29TY76Z2rM5mHXA On Wed, 8 Mar 2017, Leonard den Ottolander wrote: > If you don't feel the limit of 4096 arguments is sufficient please > provide us with an example where that limit is insufficient. mv * /some/where/ (in a directory with many files). Linking thousands of .o files into a large program or putting them in a .a file in a single command. Constructing a make command line with thousands of -o options to mark files as considered to be old (a real case where I ran into the old 128k limit when it was there). There are of course workarounds for these - but "the number of files in the source + build trees of a very large project" seems a better way of judging limits than 4096. -- Joseph S. Myers joseph-qD8j1LwMmJjtCj0u4l0SBw@public.gmane.org ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying 2017-03-08 18:18 ` Leonard den Ottolander 2017-03-08 18:21 ` Leonard den Ottolander 2017-03-08 20:47 ` Joseph Myers @ 2017-03-08 21:05 ` Carlos O'Donell [not found] ` <f16cd7f8-f996-cf66-d640-50b0ccee06c7-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2 siblings, 1 reply; 17+ messages in thread From: Carlos O'Donell @ 2017-03-08 21:05 UTC (permalink / raw) To: Leonard den Ottolander, linux-api-u79uwXL29TY76Z2rM5mHXA On 03/08/2017 01:18 PM, Leonard den Ottolander wrote: > On Wed, 2017-03-08 at 12:54 -0500, Carlos O'Donell wrote: >> In glibc we limit setuid applications, for example sanitizing their >> environment where it would cause problems or alter behaviour in >> unintended ways. >> >> Can we avoid imposing a limit on all applications? > > Not imposing a limit - btw, 0x7FFFFFFF *is* a limit albeit a ridiculous > and dangerously large limit - is a bad idea, because it allows the > aforementioned "heap-spraying" which is a serious attack vector. > > Note that 128KiB * 4096 arguments still adds up to 512MiB!!! > > If you don't feel the limit of 4096 arguments is sufficient please > provide us with an example where that limit is insufficient. (a) Justification. A good design should not unduly limit what users can do without a good justification. It is my opinion that it is not enough data to say that a kernel build is sufficient to justify a limit that will affect all applications. Minimally I'd expect you to run with such a kernel patch through an entire distro build cycle to see if anything else breaks. For example reaching out to the Fedora or SUSE teams to test the patch in Rawhide or Tumbleweed. What if 4096 is too much? Why not lower? You could try using systemtap or dtrace and running a distro start to finish and auditing the mean values you see (see (c) for a discussion on ensuring the userspace tooling remains correct after this change). (b) Attack vector. The privilege escalation in your example is caused by a fault in a setuid application. Can we impose stricter limits on setuid binaries instead of on all binaries? In glibc for example we do not allow certain environment variables to impact the behaviour of setuid binaries e.g. MALLOC_PERTURB_ env var is ignored in secure mode for setuid binaries (which might be used for similar heap-spraying). Can the kernel similarly enforce a different MAX_ARG_STRINGS for setuid? (c) What limit is appropriate? No limit is appropriate. Users should be able to use all of their system resources to accomplish whatever task they need to get done. Yes, portable applications have limits e.g. ARG_MAX, sysconf(_SC_ARG_MAX), xargs --show-limits, getconf ARG_MAX etc. We should make sure that any limit we set does not conflict with current implementations and standards conformance. Yes, limits should be balanced against security issues, and for setuid binaries I agree, but you propose a blanket limit, which is why I'm asking the question again: Can we impose stricter limits on setuid binaries instead of on all binaries? -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <f16cd7f8-f996-cf66-d640-50b0ccee06c7-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying [not found] ` <f16cd7f8-f996-cf66-d640-50b0ccee06c7-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2017-03-09 14:04 ` Leonard den Ottolander 2017-03-09 14:35 ` Carlos O'Donell 2017-03-09 14:14 ` Leonard den Ottolander 1 sibling, 1 reply; 17+ messages in thread From: Leonard den Ottolander @ 2017-03-09 14:04 UTC (permalink / raw) To: linux-api-u79uwXL29TY76Z2rM5mHXA On Wed, 2017-03-08 at 16:05 -0500, Carlos O'Donell wrote: > On 03/08/2017 01:18 PM, Leonard den Ottolander wrote: > > Note that 128KiB * 4096 arguments still adds up to 512MiB!!! > > > > If you don't feel the limit of 4096 arguments is sufficient please > > provide us with an example where that limit is insufficient. > > (a) Justification. > > A good design should not unduly limit what users can do without a good > justification. The justification is that the excessive value of the MAX_ARG_STRINGS allows heap spraying on 32 bit processes. Or more precisely, the fact that the multiplication of MAX_ARG_STRINGS and MAX_ARG_STRLEN being equal to or larger than the 2^32 allows heap spraying. > It is my opinion that it is not enough data to say that a kernel build > is sufficient to justify a limit that will affect all applications. > > Minimally I'd expect you to run with such a kernel patch through an entire > distro build cycle to see if anything else breaks. I don't think that's really necessary. We can argue whether that value should be a bit bigger, but a limit is necessary to disable the very serious attack vector that heap spraying is. In any binary, not just setuid ones. It's a jumping board to launch any attack via the heap sprayed binary. > What if 4096 is too much? Why not lower? Well, building that C7 kernel didn't work with 2048, it choked on a make file with something like a little over 3000 -o options. I chose 4096 as it was sufficient for what I considered to be one of the heaviest use cases. The problem is that the multiplication of MAX_ARG_STRLEN and MAX_ARG_STRINGS should stay below 4GiB to disable the heap spraying. This means that if f.e. both MAX_ARG_STRINGS and MAX_ARG_STRLEN are 65536 the multiplication is 4GiB and heap spraying is possible. Of course an extra value limiting this multiplier could be added (MAX_ARG_STRSLEN along the lines of MAX_ARG_PAGES) and used in exec.c to limit the total amount of reserved memory. This would allow for the multiplication of MAX_ARG_STRINGS and MAX_ARG_STRLEN to go beyond 2^32. And it would save a lot of memory because even with my conservative values the amount of memory reserved for options is still 65536 x 4096 = 256MiB. > You could try using systemtap > or dtrace and running a distro start to finish and auditing the mean > values you see (see (c) for a discussion on ensuring the userspace tooling > remains correct after this change). I have not been dtracing for, but I've not encountered any issue with kernels using MAX_ARG_STRLEN 65536 and MAX_ARG_STRINGS 4096 so far (running a.o. on this desktop). > (b) Attack vector. > > The privilege escalation in your example is caused by a fault in a > setuid application. The orthogonality of attack vectors seems to be lost on many I speak on the issue. It means that every vector has to be judged individually. The fact that the binary in the example is setuid is orthogonal/irrelevant to the fact that the high value of MAX_ARG_STRINGS combined with memory leaks in executables option parsing allow heap spraying. The heap spraying in itself is a very serious vector that allows launching any attack via the sprayed heap whether or not the involved binary is setuid. > Can we impose stricter limits on setuid binaries instead of on all > binaries? Because an zero day can also be launched via a heap sprayed non setuid binary like pkcheck. > In glibc for example we do not allow certain environment variables to > impact the behaviour of setuid binaries e.g. MALLOC_PERTURB_ env var > is ignored in secure mode for setuid binaries (which might be used > for similar heap-spraying). This is all very nice but it did not block the attack described in the article I linked. Your solution sounds rather convoluted. With a sensible value for MAX_ARG_STRINGS all this working around the real issue is unnecessary: The heap will no longer collide into the stack of any binary that does not sanitize its argument list. > (c) What limit is appropriate? > > No limit is appropriate. Users should be able to use all of their system > resources to accomplish whatever task they need to get done. Oh the poor user. So you rather leave the user exposed to attacks that leverage heap spraying than imposing a limit? The point is that this value (the multiplication of both really) allows an adversary to launch any attack in his inventory using a heap sprayed binary. That is not a situation you want to prolong. Regards, Leonard. -- mount -t life -o ro /dev/dna /genetic/research ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying 2017-03-09 14:04 ` Leonard den Ottolander @ 2017-03-09 14:35 ` Carlos O'Donell 0 siblings, 0 replies; 17+ messages in thread From: Carlos O'Donell @ 2017-03-09 14:35 UTC (permalink / raw) To: Leonard den Ottolander, linux-api-u79uwXL29TY76Z2rM5mHXA On 03/09/2017 09:04 AM, Leonard den Ottolander wrote: > On Wed, 2017-03-08 at 16:05 -0500, Carlos O'Donell wrote: >> On 03/08/2017 01:18 PM, Leonard den Ottolander wrote: >>> Note that 128KiB * 4096 arguments still adds up to 512MiB!!! >>> >>> If you don't feel the limit of 4096 arguments is sufficient please >>> provide us with an example where that limit is insufficient. >> >> (a) Justification. >> >> A good design should not unduly limit what users can do without a good >> justification. > > The justification is that the excessive value of the MAX_ARG_STRINGS > allows heap spraying on 32 bit processes. Or more precisely, the fact > that the multiplication of MAX_ARG_STRINGS and MAX_ARG_STRLEN being > equal to or larger than the 2^32 allows heap spraying. > >> It is my opinion that it is not enough data to say that a kernel build >> is sufficient to justify a limit that will affect all applications. >> >> Minimally I'd expect you to run with such a kernel patch through an entire >> distro build cycle to see if anything else breaks. > > I don't think that's really necessary. We can argue whether that value > should be a bit bigger, but a limit is necessary to disable the very > serious attack vector that heap spraying is. In any binary, not just > setuid ones. It's a jumping board to launch any attack via the heap > sprayed binary. It is my opinion that your justification is insufficient to limit all of userspace to the newer more restrictive limit without any remedy should existing build systems start to fail. One could argue the use of @file support in the compiler, static linker, and assembler might help, but that doesn't fix all issues and could be costly to re-architect such build systems. We should pursue other avenues to limit the attack vector first. > Of course an extra value limiting this multiplier could be added > (MAX_ARG_STRSLEN along the lines of MAX_ARG_PAGES) and used in exec.c to > limit the total amount of reserved memory. This would allow for the > multiplication of MAX_ARG_STRINGS and MAX_ARG_STRLEN to go beyond 2^32. This sounds like a much better option, particularly if as a remedy the system administrator can control the max pages allowed for this to a configurable limit. > I have not been dtracing for, but I've not encountered any issue with > kernels using MAX_ARG_STRLEN 65536 and MAX_ARG_STRINGS 4096 so far > (running a.o. on this desktop). This is insufficient research into the problem to justify lowering the limit. >> (c) What limit is appropriate? >> >> No limit is appropriate. Users should be able to use all of their system >> resources to accomplish whatever task they need to get done. > > Oh the poor user. So you rather leave the user exposed to attacks that > leverage heap spraying than imposing a limit? There is a balance between keeping large build systems functional and limiting arguments. My position is that I would rather harden malloc metadata than limit argument lengths. Florian Weimer on my team (Red Hat glibc team) is already looking at this: https://sourceware.org/ml/libc-alpha/2016-10/msg00531.html This would force attackers to have to find entirely new ways to attack the heap metadata. > The point is that this value (the multiplication of both really) allows > an adversary to launch any attack in his inventory using a heap sprayed > binary. That is not a situation you want to prolong. This is hyperbole. You have to take a common sense approach to closing these attack vectors and balance them against system usability. -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying [not found] ` <f16cd7f8-f996-cf66-d640-50b0ccee06c7-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2017-03-09 14:04 ` Leonard den Ottolander @ 2017-03-09 14:14 ` Leonard den Ottolander 2017-03-09 20:34 ` Carlos O'Donell 2017-03-09 23:10 ` Joseph Myers 1 sibling, 2 replies; 17+ messages in thread From: Leonard den Ottolander @ 2017-03-09 14:14 UTC (permalink / raw) To: linux-api-u79uwXL29TY76Z2rM5mHXA --- a/include/uapi/linux/binfmts.h 2016-11-23 21:02:31.000000000 +0100 +++ b/include/uapi/linux/binfmts.h 2017-03-09 01:52:14.716319950 +0100 @@ -9,10 +9,15 @@ struct pt_regs; * These are the maximum length and maximum number of strings passed to the * execve() system call. MAX_ARG_STRLEN is essentially random but serves to * prevent the kernel from being unduly impacted by misaddressed pointers. - * MAX_ARG_STRINGS is chosen to fit in a signed 32-bit integer. + * MAX_ARG_STRINGS * MAX_ARG_STRLEN should be smaller than the 4GiB + * address space on 32 bit to avoid heap spraying. + * MAX_ARG_STRSLEN to the rescue - the MAX_ARG_PAGES concept was there + * for a reason. + * We can now safely increase STRINGS * STRLEN beyond 4GiB if need be. */ -#define MAX_ARG_STRLEN (PAGE_SIZE * 32) -#define MAX_ARG_STRINGS 0x7FFFFFFF +#define MAX_ARG_STRSLEN 262144 +#define MAX_ARG_STRLEN 65536 +#define MAX_ARG_STRINGS 4096 /* sizeof(linux_binprm->buf) */ #define BINPRM_BUF_SIZE 128 --- a/fs/exec.c 2017-02-20 08:04:43.000000000 +0100 +++ b/fs/exec.c 2017-03-09 01:54:25.931476370 +0100 @@ -459,6 +459,7 @@ static int copy_strings(int argc, struct char *kaddr = NULL; unsigned long kpos = 0; int ret; + int total_bytes = 0; while (argc-- > 0) { const char __user *str; @@ -478,6 +479,11 @@ static int copy_strings(int argc, struct if (!valid_arg_len(bprm, len)) goto out; + /* -E2BIG is fine for now */ + total_bytes += len; + if (total_bytes > MAX_ARG_STRSLEN) + goto out; + /* We're going to work our way backwords. */ pos = bprm->p; str += len; I've successfully built a kernel on a system with a kernel using above values. As we have now introduced a safeguard (MAX_ARG_STRSLEN capping the total amount of memory reserved) the values of MAX_ARG_STRLEN and MAX_ARG_STRINGS can be increased beyond where their multiplication reaches 2^32. So if we really want to support lets say users having directories of 128k files we can now safely set MAX_ARG_STRINGS to 131072 and assuming an average file name length of 32 set MAX_ARG_STRSLEN to 4194304. Seems a little excessive to me for a default, but it is now safe. Regards, Leonard. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying 2017-03-09 14:14 ` Leonard den Ottolander @ 2017-03-09 20:34 ` Carlos O'Donell [not found] ` <81d8e14e-e110-4b96-5d45-8bb3b56f4866-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2017-03-09 23:10 ` Joseph Myers 1 sibling, 1 reply; 17+ messages in thread From: Carlos O'Donell @ 2017-03-09 20:34 UTC (permalink / raw) To: Leonard den Ottolander, linux-api-u79uwXL29TY76Z2rM5mHXA On 03/09/2017 09:14 AM, Leonard den Ottolander wrote: > --- a/include/uapi/linux/binfmts.h 2016-11-23 21:02:31.000000000 +0100 > +++ b/include/uapi/linux/binfmts.h 2017-03-09 01:52:14.716319950 +0100 > @@ -9,10 +9,15 @@ struct pt_regs; > * These are the maximum length and maximum number of strings passed to the > * execve() system call. MAX_ARG_STRLEN is essentially random but serves to > * prevent the kernel from being unduly impacted by misaddressed pointers. > - * MAX_ARG_STRINGS is chosen to fit in a signed 32-bit integer. > + * MAX_ARG_STRINGS * MAX_ARG_STRLEN should be smaller than the 4GiB > + * address space on 32 bit to avoid heap spraying. OK > + * MAX_ARG_STRSLEN to the rescue - the MAX_ARG_PAGES concept was there > + * for a reason. Why not just use MAX_ARG_PAGES? Define it in include/linux/binfmts.h for !CONFIG_MMU case. > + * We can now safely increase STRINGS * STRLEN beyond 4GiB if need be. > */ > -#define MAX_ARG_STRLEN (PAGE_SIZE * 32) > -#define MAX_ARG_STRINGS 0x7FFFFFFF > +#define MAX_ARG_STRSLEN 262144 You don't need this if you define MAX_ARG_PAGES. > +#define MAX_ARG_STRLEN 65536 > +#define MAX_ARG_STRINGS 4096 These should be left untouched at their original values. > /* sizeof(linux_binprm->buf) */ > #define BINPRM_BUF_SIZE 128 > --- a/fs/exec.c 2017-02-20 08:04:43.000000000 +0100 > +++ b/fs/exec.c 2017-03-09 01:54:25.931476370 +0100 > @@ -459,6 +459,7 @@ static int copy_strings(int argc, struct > char *kaddr = NULL; > unsigned long kpos = 0; > int ret; > + int total_bytes = 0; > > while (argc-- > 0) { > const char __user *str; > @@ -478,6 +479,11 @@ static int copy_strings(int argc, struct > if (!valid_arg_len(bprm, len)) > goto out; > > + /* -E2BIG is fine for now */ > + total_bytes += len; > + if (total_bytes > MAX_ARG_STRSLEN) Should be PAGE_SIZE * MAX_ARG_PAGES, similar to the !CONFIG_MMU case. > + goto out; > + > /* We're going to work our way backwords. */ > pos = bprm->p; > str += len; > > I've successfully built a kernel on a system with a kernel using above > values. > > As we have now introduced a safeguard (MAX_ARG_STRSLEN capping the total > amount of memory reserved) the values of MAX_ARG_STRLEN and > MAX_ARG_STRINGS can be increased beyond where their multiplication > reaches 2^32. > > So if we really want to support lets say users having directories of > 128k files we can now safely set MAX_ARG_STRINGS to 131072 and assuming > an average file name length of 32 set MAX_ARG_STRSLEN to 4194304. > > Seems a little excessive to me for a default, but it is now safe. We're getting closer to a solution. There is still no justification for the value of MAX_ARG_PAGES though. My objection that build systems will break still stands. The point of memory is to be used. A reasonable limit might be 1/4 of the virtual address space used for argument pages though. -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <81d8e14e-e110-4b96-5d45-8bb3b56f4866-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying [not found] ` <81d8e14e-e110-4b96-5d45-8bb3b56f4866-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2017-03-10 12:10 ` Leonard den Ottolander 2017-03-14 0:51 ` Carlos O'Donell 0 siblings, 1 reply; 17+ messages in thread From: Leonard den Ottolander @ 2017-03-10 12:10 UTC (permalink / raw) To: linux-api-u79uwXL29TY76Z2rM5mHXA On Thu, 2017-03-09 at 15:34 -0500, Carlos O'Donell wrote: > Why not just use MAX_ARG_PAGES? Well it is really just a name, but MAX_ARG_PAGES is in use for non MMU. So to avoid name collisions a different name seems appropriate. > On 03/09/2017 09:14 AM, Leonard den Ottolander wrote: > > +#define MAX_ARG_STRLEN 65536 > > +#define MAX_ARG_STRINGS 4096 > > These should be left untouched at their original values. They could actually be removed unless something external relies on them. But not to disturb too much they can stay around without causing issues. > > + if (total_bytes > MAX_ARG_STRSLEN) > > Should be PAGE_SIZE * MAX_ARG_PAGES, similar to the !CONFIG_MMU case. The use of PAGE_SIZE is arbitrary in this case. There is no reason why the total amount of memory used for arguments should be a multitude of PAGE_SIZE. Something like MAX_ARG_BYTES is just fine. As I mentioned above, reusing MAX_ARG_PAGES might not be a good idea as it might cause collisions with the non MMU case. > > I've successfully built a kernel on a system with a kernel using above > > values. > > > > As we have now introduced a safeguard (MAX_ARG_STRSLEN capping the total > > amount of memory reserved) the values of MAX_ARG_STRLEN and > > MAX_ARG_STRINGS can be increased beyond where their multiplication > > reaches 2^32. > > > > So if we really want to support lets say users having directories of > > 128k files we can now safely set MAX_ARG_STRINGS to 131072 and assuming > > an average file name length of 32 set MAX_ARG_STRSLEN to 4194304. > > > > Seems a little excessive to me for a default, but it is now safe. > There is still no justification for the value of MAX_ARG_PAGES though. The value for MAX_ARG_PAGES was chosen to allow users to specify large command lines, for example when building a kernel or other large project, or handling many files in a directory. 131072 bytes (32 pages) did the trick in most cases 15 years ago. Only in special circumstances did that value cause trouble, as you can see in the Linux Journal article I linked. A value of 262144 bytes (64 pages) causes no obvious problems on my systems today. > My objection that build systems will break still stands. Well, you can keep repeating that argument without showing a use case where the values I chose cause trouble. It's hard for me to argue against conjecture. As I have pointed out I can build a kernel on CentOS-7 with a kernel using 262144 for MAX_ARG_STRSLEN. If you have a heavier use case that you want to support then describe that use case and tell us how much memory it needs for command line options. I have come up with my values after quite some experimentation. > The point of memory is to be used. Not if it is dangerous for the user. Limits are there not to annoy the user but to protect the user from the system misbehaving. I would call heap spraying, an attack vector that allows launching any exploit via the affected (memory leaking) binary as very serious misbehaviour. > A reasonable limit might be 1/4 of the virtual address space used for > argument pages though. A whopping gigabyte on 32 bit systems for arguments passed to an executable. That sounds excessive. A use case of "ls *"-ing 128k files with 32 byte names adds up to 4 MiB. That is 16 times what you need to build a kernel. That sounds sufficient don't you think? And again, if not, show us a use case. Regards, Leonard. -- mount -t life -o ro /dev/dna /genetic/research ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying 2017-03-10 12:10 ` Leonard den Ottolander @ 2017-03-14 0:51 ` Carlos O'Donell [not found] ` <b736f01f-ef0a-56de-bf57-c6d3d74262a4-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Carlos O'Donell @ 2017-03-14 0:51 UTC (permalink / raw) To: Leonard den Ottolander, linux-api-u79uwXL29TY76Z2rM5mHXA On 03/10/2017 07:10 AM, Leonard den Ottolander wrote: > A whopping gigabyte on 32 bit systems for arguments passed to an > executable. That sounds excessive. A use case of "ls *"-ing 128k files > with 32 byte names adds up to 4 MiB. That is 16 times what you need to > build a kernel. That sounds sufficient don't you think? And again, if > not, show us a use case. You have already been given use cases. * Mail spool directory with millions of files. * Large and complex build system with potentially long paths. Artificial limits are always going to cause problems, particularly when going from a higher limit to a lower limit. Your present justification is simply that nobody could have use for lots of arguments, which is not true, and that heap spraying needs to be stopped, which may be right but may not be the place to start. We should investigate other ways in which we can tackle the manner in which the zero day was carried out (like malloc metadata hardening for the default glibc allocator). Fundamentally our disagreement is about the risk versus value of the code in the kernel. Because we disagree about risk and value we will disagree about the action that needs to be taken. You have now been given the expert opinion of Joseph Myers and myself, so take that as you will. Deployed existing code stands, and the burden of proof is on _you_ to justify changing the code, not on me to justify all the use cases that users might have for the current implementation. Please invite more experts, security or otherwise, to come discuss on linux-api, the relative merits of _where_ we should be fixing attacks of this kind. -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <b736f01f-ef0a-56de-bf57-c6d3d74262a4-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying [not found] ` <b736f01f-ef0a-56de-bf57-c6d3d74262a4-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2017-03-17 13:12 ` Leonard den Ottolander 0 siblings, 0 replies; 17+ messages in thread From: Leonard den Ottolander @ 2017-03-17 13:12 UTC (permalink / raw) To: linux-api-u79uwXL29TY76Z2rM5mHXA On Mon, 2017-03-13 at 20:51 -0400, Carlos O'Donell wrote: > On 03/10/2017 07:10 AM, Leonard den Ottolander wrote: > > A whopping gigabyte on 32 bit systems for arguments passed to an > > executable. That sounds excessive. A use case of "ls *"-ing 128k files > > with 32 byte names adds up to 4 MiB. That is 16 times what you need to > > build a kernel. That sounds sufficient don't you think? And again, if > > not, show us a use case. > > You have already been given use cases. > > * Mail spool directory with millions of files. That is quite an excessive use case to bring up to counter my effort to get a serious attack vector, heap spraying, fixed. Also not that ls on such a directory will work fine, it's only when you start to use wildcards that you run into limits. Easily fixed by someone with only a basic knowledge of scripting. > * Large and complex build system with potentially long paths. I have given you a concrete example: You can build a kernel on CentOS-7 with a kernel built with these values. You still have not shown a counter example. Perhaps the build of glibc fails with such values? Firefox? MySQL/MariaDB? You keep repeating conjectured arguments to counter my concrete example. Please provide a real case where a total of 256KiB for command line arguments breaks anything in real life. > Artificial limits are always going to cause problems, particularly when > going from a higher limit to a lower limit. 15 years ago a total of 128KiB arguments was fine. I can imagine we need a multitude of that by now, but for the use case I present 256KiB seems plenty. As I have pointed out all systems use artificial limits everywhere, not to annoy the user, but to protect the users system against (dangerously) misbehaving. > Your present justification is simply that nobody could have use for > lots of arguments, which is not true, and that heap spraying needs to be > stopped, which may be right but may not be the place to start. No, I have presented you a concrete use case that I believe to be at the top end of what one would need for the total amount of argument data. You have still not presented a serious counter example. > You have now been given the expert opinion of Joseph Myers and myself, > so take that as you will. Who exactly is the judge of that qualification? Are you trying to disqualify me by using such terms and applying them to yourselves? > Deployed existing code stands, and the burden of proof is on _you_ to > justify changing the code, not on me to justify all the use cases that > users might have for the current implementation. The example I have given you https://googleprojectzero.blogspot.nl/2014/08/the-poisoned-nul-byte-2014-edition.html clearly illustrates the viability of attacking a system with heap spraying and its potency as an attack vector. > Please invite more experts, security or otherwise, to come discuss on > linux-api, the relative merits of _where_ we should be fixing attacks > of this kind. Apparently the amount of people chipping in is more important to you than the value of the arguments. You being an employee of a large company will always be able to bring out more people to argue your side of the argument. That in itself does not make your arguments more valid though. The change to split that single value MAX_ARG_PAGES into two (MAX_ARG_STRINGS & MAX_ARG_STRLEN) when MMU was implemented has not been justified. It now turns out to have been a dangerous decision as the multiplication of the two quickly brings us in the danger zone where the entire heap can be initialized by an attacker (malevolent local user, or someone cracking f.e. apache running on a 32 bit ABI). Instead of trying to disqualify me and my arguments with cheap rhetoric you might run some tests with the value(s) I provided so we can establish a sensible limit for MAX_ARG_STRINGS * MAX_ARG_STRLEN or its (to be restored) cap "MAX_ARG_BYTES" and fix the dangerous attack venue that heap spraying is. Regards, Leonard. -- mount -t life -o ro /dev/dna /genetic/research ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying 2017-03-09 14:14 ` Leonard den Ottolander 2017-03-09 20:34 ` Carlos O'Donell @ 2017-03-09 23:10 ` Joseph Myers [not found] ` <alpine.DEB.2.20.1703092304110.23273-9YEB1lltEqivcGRMvF24k2I39yigxGEX@public.gmane.org> 1 sibling, 1 reply; 17+ messages in thread From: Joseph Myers @ 2017-03-09 23:10 UTC (permalink / raw) To: Leonard den Ottolander; +Cc: linux-api-u79uwXL29TY76Z2rM5mHXA On Thu, 9 Mar 2017, Leonard den Ottolander wrote: > So if we really want to support lets say users having directories of > 128k files we can now safely set MAX_ARG_STRINGS to 131072 and assuming > an average file name length of 32 set MAX_ARG_STRSLEN to 4194304. I think 128k files is too small. E.g. I have nearly 4 million archived emails here, and I'm sure many people have many more than that, and it's common to store emails one per file in a large directory (although I'm not storing them like that), and I think normal command-line tools ought to be able to take all the files in such a directory on the command line at once. I think it's best not to limit the number of arguments beyond the limit implied by argc being an int, and no individual object (e.g. the argv array) taking up half or more of the address space, along possibly with a limit on the proportion of the whole address space taken up by (command-line arguments + environment variables + all the pointers to those). -- Joseph S. Myers joseph-qD8j1LwMmJjtCj0u4l0SBw@public.gmane.org ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <alpine.DEB.2.20.1703092304110.23273-9YEB1lltEqivcGRMvF24k2I39yigxGEX@public.gmane.org>]
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying [not found] ` <alpine.DEB.2.20.1703092304110.23273-9YEB1lltEqivcGRMvF24k2I39yigxGEX@public.gmane.org> @ 2017-03-10 0:01 ` Carlos O'Donell 0 siblings, 0 replies; 17+ messages in thread From: Carlos O'Donell @ 2017-03-10 0:01 UTC (permalink / raw) To: Joseph Myers, Leonard den Ottolander; +Cc: linux-api-u79uwXL29TY76Z2rM5mHXA On 03/09/2017 06:10 PM, Joseph Myers wrote: > On Thu, 9 Mar 2017, Leonard den Ottolander wrote: > >> So if we really want to support lets say users having directories of >> 128k files we can now safely set MAX_ARG_STRINGS to 131072 and assuming >> an average file name length of 32 set MAX_ARG_STRSLEN to 4194304. > > I think 128k files is too small. E.g. I have nearly 4 million archived > emails here, and I'm sure many people have many more than that, and it's > common to store emails one per file in a large directory (although I'm not > storing them like that), and I think normal command-line tools ought to be > able to take all the files in such a directory on the command line at > once. > > I think it's best not to limit the number of arguments beyond the limit > implied by argc being an int, and no individual object (e.g. the argv > array) taking up half or more of the address space, along possibly with a > limit on the proportion of the whole address space taken up by > (command-line arguments + environment variables + all the pointers to > those). Agreed. -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying [not found] ` <f7f9f60b-0f39-7dce-1778-3aa40ba198ef-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2017-03-08 18:18 ` Leonard den Ottolander @ 2017-03-08 18:48 ` Leonard den Ottolander 1 sibling, 0 replies; 17+ messages in thread From: Leonard den Ottolander @ 2017-03-08 18:48 UTC (permalink / raw) To: linux-api-u79uwXL29TY76Z2rM5mHXA On Wed, 2017-03-08 at 12:54 -0500, Carlos O'Donell wrote: > In glibc we limit setuid applications, for example sanitizing their > environment where it would cause problems or alter behaviour in > unintended ways. Please explain what these limitations are, and when they were imposed, as in the article https://googleprojectzero.blogspot.nl/2014/08/the-poisoned-nul-byte-2014-edition.html the author is actually using a setuid binary (pkexec) and clearly not running into any limitations with that particular exploit. Also note that heap spraying can happen in any binary that has memory leaks in its option parsing. pkexec.c and pkcheck.c are known to suffer such issues, but other binaries could be affected. Setting MAX_ARG_STRINGS to a sensible value significantly reduces the impact of such heap spraying. Regards, Leonard. -- mount -t life -o ro /dev/dna /genetic/research ^ permalink raw reply [flat|nested] 17+ messages in thread
* binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying @ 2017-03-06 15:29 Leonard den Ottolander 0 siblings, 0 replies; 17+ messages in thread From: Leonard den Ottolander @ 2017-03-06 15:29 UTC (permalink / raw) To: linux-kernel In the article https://googleprojectzero.blogspot.nl/2014/08/the-poisoned-nul-byte-2014-edition.html the author describes launching an attack on an off by one NUL byte bug in glibc. He explains that both the memory leak in the option parsing of PolKits pkexec.c and the excessive value of MAX_ARG_STRINGS in the kernels include/uapi/linux/binfmts.h (#define MAX_ARG_STRINGS 0x7FFFFFFF) enabled his attack. The rationale for that excessive value is a bit odd: "MAX_ARG_STRINGS is chosen to fit in a signed 32-bit integer." https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/binfmts.h?id=607ca46e97a1b6594b29647d98a32d545c24bdff The fact that the integer is so large makes that on a 32 bit platform the entire heap can be initialized with values the attacker provides, given the memory leak in the option parsing of pkexec.c, an approach the author calls "heap spraying". (Note that two similar memory leaks exist in the option parsing of pkexecs sibling pkcheck.c, so pkcheck will also allow an attacker to heap spray its entire memory on a 32 bit system.) If you compare http://www.linuxjournal.com/article/6060 you will see that the TOTAL amount of memory reserved for command line arguments used to be 32 pages, i.e. 128KiB (#define MAX_ARG_PAGES 32). The amount of memory reserved for command line arguments in more current kernels is the multiplication of MAX_ARG_STRINGS and MAX_ARG_STRLEN. Both the memory leaks in pkexec.c and pkcheck.c seem very severe, but their impact would be much less if MAX_ARG_STRINGS would hold a sensible value. After some experimentation with $ rpmbuild -ba kernel.spec on CentOS-7 I've come up with values that allow this kernel compilation. As this kind of build is probably one of the most demanding actions in relation to MAX_ARG_STRINGS and MAX_ARG_STRLEN I believe below values to be sensible and much safer defaults: #define MAX_ARG_STRLEN 65536 #define MAX_ARG_STRINGS 4096 Please let me know if this needs to be filed as a bug somewhere. Regards, Leonard. -- mount -t life -o ro /dev/dna /genetic/research ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-03-17 13:12 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-07 14:44 binfmts.h MAX_ARG_STRINGS excessive value allows heap spraying Leonard den Ottolander 2017-03-08 17:54 ` Carlos O'Donell [not found] ` <f7f9f60b-0f39-7dce-1778-3aa40ba198ef-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2017-03-08 18:18 ` Leonard den Ottolander 2017-03-08 18:21 ` Leonard den Ottolander 2017-03-08 20:47 ` Joseph Myers 2017-03-08 21:05 ` Carlos O'Donell [not found] ` <f16cd7f8-f996-cf66-d640-50b0ccee06c7-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2017-03-09 14:04 ` Leonard den Ottolander 2017-03-09 14:35 ` Carlos O'Donell 2017-03-09 14:14 ` Leonard den Ottolander 2017-03-09 20:34 ` Carlos O'Donell [not found] ` <81d8e14e-e110-4b96-5d45-8bb3b56f4866-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2017-03-10 12:10 ` Leonard den Ottolander 2017-03-14 0:51 ` Carlos O'Donell [not found] ` <b736f01f-ef0a-56de-bf57-c6d3d74262a4-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2017-03-17 13:12 ` Leonard den Ottolander 2017-03-09 23:10 ` Joseph Myers [not found] ` <alpine.DEB.2.20.1703092304110.23273-9YEB1lltEqivcGRMvF24k2I39yigxGEX@public.gmane.org> 2017-03-10 0:01 ` Carlos O'Donell 2017-03-08 18:48 ` Leonard den Ottolander -- strict thread matches above, loose matches on Subject: below -- 2017-03-06 15:29 Leonard den Ottolander
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.