| Message ID | 20150214173350.GA18393@www.outflux.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/14/2015 09:33 AM, Kees Cook wrote: > From: Hector Marco-Gisbert <hecmargi@upv.es> > > The issue is that the stack for processes is not properly randomized on 64 bit > architectures due to an integer overflow. > > The affected function is randomize_stack_top() in file "fs/binfmt_elf.c": > > static unsigned long randomize_stack_top(unsigned long stack_top) > { > unsigned int random_variable = 0; > > if ((current->flags & PF_RANDOMIZE) && > !(current->personality & ADDR_NO_RANDOMIZE)) { > random_variable = get_random_int() & STACK_RND_MASK; > random_variable <<= PAGE_SHIFT; > } > return PAGE_ALIGN(stack_top) + random_variable; > return PAGE_ALIGN(stack_top) - random_variable; > } > > Note that, it declares the "random_variable" variable as "unsigned int". Since > the result of the shifting operation between STACK_RND_MASK (which is > 0x3fffff on x86_64, 22 bits) and PAGE_SHIFT (which is 12 on x86_64): > > random_variable <<= PAGE_SHIFT; > > then the two leftmost bits are dropped when storing the result in the > "random_variable". This variable shall be at least 34 bits long to hold the > (22+12) result. > > These two dropped bits have an impact on the entropy of process stack. > Concretely, the total stack entropy is reduced by four: from 2^28 to 2^30 (One > fourth of expected entropy). > > This patch restores back the entropy by correcting the types involved in the > operations in the functions randomize_stack_top() and stack_maxrandom_size(). > > The successful fix can be tested with: > $ for i in `seq 1 10`; do cat /proc/self/maps | grep stack; done > 7ffeda566000-7ffeda587000 rw-p 00000000 00:00 0 [stack] > 7fff5a332000-7fff5a353000 rw-p 00000000 00:00 0 [stack] > 7ffcdb7a1000-7ffcdb7c2000 rw-p 00000000 00:00 0 [stack] > 7ffd5e2c4000-7ffd5e2e5000 rw-p 00000000 00:00 0 [stack] > ... > > Once corrected, the leading bytes should be between 7ffc and 7fff, rather > than always being 7fff. > > CVE-2015-1593 Awesome. So the vdso randomization *and* the stack randomization implementations were buggy. Anyone want to check the mmap and brk randomization implementations? --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 16, 2015 at 12:49 PM, Andy Lutomirski <luto@amacapital.net> wrote: > On 02/14/2015 09:33 AM, Kees Cook wrote: >> >> From: Hector Marco-Gisbert <hecmargi@upv.es> >> >> The issue is that the stack for processes is not properly randomized on 64 >> bit >> architectures due to an integer overflow. >> >> The affected function is randomize_stack_top() in file "fs/binfmt_elf.c": >> >> static unsigned long randomize_stack_top(unsigned long stack_top) >> { >> unsigned int random_variable = 0; >> >> if ((current->flags & PF_RANDOMIZE) && >> !(current->personality & ADDR_NO_RANDOMIZE)) { >> random_variable = get_random_int() & STACK_RND_MASK; >> random_variable <<= PAGE_SHIFT; >> } >> return PAGE_ALIGN(stack_top) + random_variable; >> return PAGE_ALIGN(stack_top) - random_variable; >> } >> >> Note that, it declares the "random_variable" variable as "unsigned int". >> Since >> the result of the shifting operation between STACK_RND_MASK (which is >> 0x3fffff on x86_64, 22 bits) and PAGE_SHIFT (which is 12 on x86_64): >> >> random_variable <<= PAGE_SHIFT; >> >> then the two leftmost bits are dropped when storing the result in the >> "random_variable". This variable shall be at least 34 bits long to hold >> the >> (22+12) result. >> >> These two dropped bits have an impact on the entropy of process stack. >> Concretely, the total stack entropy is reduced by four: from 2^28 to 2^30 >> (One >> fourth of expected entropy). >> >> This patch restores back the entropy by correcting the types involved in >> the >> operations in the functions randomize_stack_top() and >> stack_maxrandom_size(). >> >> The successful fix can be tested with: >> $ for i in `seq 1 10`; do cat /proc/self/maps | grep stack; done >> 7ffeda566000-7ffeda587000 rw-p 00000000 00:00 0 >> [stack] >> 7fff5a332000-7fff5a353000 rw-p 00000000 00:00 0 >> [stack] >> 7ffcdb7a1000-7ffcdb7c2000 rw-p 00000000 00:00 0 >> [stack] >> 7ffd5e2c4000-7ffd5e2e5000 rw-p 00000000 00:00 0 >> [stack] >> ... >> >> Once corrected, the leading bytes should be between 7ffc and 7fff, rather >> than always being 7fff. >> >> CVE-2015-1593 > > > Awesome. So the vdso randomization *and* the stack randomization > implementations were buggy. Anyone want to check the mmap and brk > randomization implementations? Both appear to use randomize_range() ... which, after looking at it, is buggy. But we've just not hit it yet. It uses get_random_int() but is modulo an unsigned long. If anything were ever to call it with a range > MAX_INT, it would truncate... -Kees > > --Andy
On Sat, Feb 14, 2015 at 09:33:50AM -0800, Kees Cook wrote: > From: Hector Marco-Gisbert <hecmargi@upv.es> > > The issue is that the stack for processes is not properly randomized on 64 bit > architectures due to an integer overflow. > > The affected function is randomize_stack_top() in file "fs/binfmt_elf.c": > > static unsigned long randomize_stack_top(unsigned long stack_top) > { > unsigned int random_variable = 0; > > if ((current->flags & PF_RANDOMIZE) && > !(current->personality & ADDR_NO_RANDOMIZE)) { > random_variable = get_random_int() & STACK_RND_MASK; > random_variable <<= PAGE_SHIFT; > } > return PAGE_ALIGN(stack_top) + random_variable; > return PAGE_ALIGN(stack_top) - random_variable; > } > > Note that, it declares the "random_variable" variable as "unsigned int". Since > the result of the shifting operation between STACK_RND_MASK (which is > 0x3fffff on x86_64, 22 bits) and PAGE_SHIFT (which is 12 on x86_64): > > random_variable <<= PAGE_SHIFT; > > then the two leftmost bits are dropped when storing the result in the > "random_variable". This variable shall be at least 34 bits long to hold the > (22+12) result. > > These two dropped bits have an impact on the entropy of process stack. > Concretely, the total stack entropy is reduced by four: from 2^28 to 2^30 (One > fourth of expected entropy). > > This patch restores back the entropy by correcting the types involved in the > operations in the functions randomize_stack_top() and stack_maxrandom_size(). > > The successful fix can be tested with: > $ for i in `seq 1 10`; do cat /proc/self/maps | grep stack; done > 7ffeda566000-7ffeda587000 rw-p 00000000 00:00 0 [stack] > 7fff5a332000-7fff5a353000 rw-p 00000000 00:00 0 [stack] > 7ffcdb7a1000-7ffcdb7c2000 rw-p 00000000 00:00 0 [stack] > 7ffd5e2c4000-7ffd5e2e5000 rw-p 00000000 00:00 0 [stack] > ... > > Once corrected, the leading bytes should be between 7ffc and 7fff, rather > than always being 7fff. > > CVE-2015-1593 > > Signed-off-by: Hector Marco-Gisbert <hecmargi@upv.es> > Signed-off-by: Ismael Ripoll <iripoll@upv.es> > [kees: rebase, fix 80 char, clean up commit message, add test example, cve] > Signed-off-by: Kees Cook <keescook@chromium.org> > Cc: stable@vger.kernel.org Ok, I'm picking this up. Do scream if someone else wants to do that, otherwise it is going to tip next week, after the merge window is over. Thanks.
On Wed, 18 Feb 2015 10:15:43 +0100 Borislav Petkov <bp@alien8.de> wrote: > > CVE-2015-1593 > > > > Signed-off-by: Hector Marco-Gisbert <hecmargi@upv.es> > > Signed-off-by: Ismael Ripoll <iripoll@upv.es> > > [kees: rebase, fix 80 char, clean up commit message, add test example, cve] > > Signed-off-by: Kees Cook <keescook@chromium.org> > > Cc: stable@vger.kernel.org > > Ok, I'm picking this up. Do scream if someone else wants to do that, I grabbed it, but shall drop my copy if it turns up in linux-next. > otherwise it is going to tip next week, after the merge window is over. It's tagged for -stable backporting, so it should go into 3.20? -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Andrew Morton <akpm@linux-foundation.org> wrote: > On Wed, 18 Feb 2015 10:15:43 +0100 Borislav Petkov <bp@alien8.de> wrote: > > > CVE-2015-1593 > > > > > > Signed-off-by: Hector Marco-Gisbert <hecmargi@upv.es> > > > Signed-off-by: Ismael Ripoll <iripoll@upv.es> > > > [kees: rebase, fix 80 char, clean up commit message, add test example, cve] > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > Cc: stable@vger.kernel.org > > > > Ok, I'm picking this up. Do scream if someone else wants to do that, > > I grabbed it, but shall drop my copy if it turns up in linux-next. > > > otherwise it is going to tip next week, after the merge window is over. > > It's tagged for -stable backporting, so it should go into 3.20? Absolutely, this is x86/urgent material, so no need to wait for -rc1 with it. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 18, 2015 at 09:19:01PM +0100, Ingo Molnar wrote: > Absolutely, this is x86/urgent material, so no need to wait for -rc1 > with it. Right, so I can start shuffling stuff to you tomorrow, this patch is in one of the pull requests. I can drop it too if preferred. I'll let you guys decide. Thanks.
* Borislav Petkov <bp@alien8.de> wrote: > On Wed, Feb 18, 2015 at 09:19:01PM +0100, Ingo Molnar wrote: > > Absolutely, this is x86/urgent material, so no need to wait for -rc1 > > with it. > > Right, so I can start shuffling stuff to you tomorrow, > this patch is in one of the pull requests. I can drop it > too if preferred. I'll let you guys decide. Tomorrow is fine to me. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c index 919b91205cd4..df4552bd239e 100644 --- a/arch/x86/mm/mmap.c +++ b/arch/x86/mm/mmap.c @@ -35,12 +35,12 @@ struct va_alignment __read_mostly va_align = { .flags = -1, }; -static unsigned int stack_maxrandom_size(void) +static unsigned long stack_maxrandom_size(void) { - unsigned int max = 0; + unsigned long max = 0; if ((current->flags & PF_RANDOMIZE) && !(current->personality & ADDR_NO_RANDOMIZE)) { - max = ((-1U) & STACK_RND_MASK) << PAGE_SHIFT; + max = ((-1UL) & STACK_RND_MASK) << PAGE_SHIFT; } return max; diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 02b16910f4c9..995986b8e36b 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -645,11 +645,12 @@ out: static unsigned long randomize_stack_top(unsigned long stack_top) { - unsigned int random_variable = 0; + unsigned long random_variable = 0; if ((current->flags & PF_RANDOMIZE) && !(current->personality & ADDR_NO_RANDOMIZE)) { - random_variable = get_random_int() & STACK_RND_MASK; + random_variable = (unsigned long) get_random_int(); + random_variable &= STACK_RND_MASK; random_variable <<= PAGE_SHIFT; } #ifdef CONFIG_STACK_GROWSUP