Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 2 Jun 2017 21:46:20 -0700
From: Kees Cook <keescook@...omium.org>
To: Rik van Riel <riel@...hat.com>
Cc: LKML <linux-kernel@...r.kernel.org>, 
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Ingo Molnar <mingo@...nel.org>, 
	Oleg Nesterov <oleg@...hat.com>, Larry Woodman <lwoodman@...hat.com>, mhocko@...e.de, 
	Daniel Micay <danielmicay@...il.com>, Will Deacon <will.deacon@....com>, 
	"benh@...nel.crashing.org" <benh@...nel.crashing.org>
Subject: Re: [PATCH 3/6] x86/mmap: properly account for
 stack randomization in mmap_base

On Fri, Jun 2, 2017 at 8:20 AM,  <riel@...hat.com> wrote:
> From: Rik van Riel <riel@...hat.com>
>
> When RLIMIT_STACK is, for example, 256MB, the current code results in
> a gap between the top of the task and mmap_base of 256MB, failing to
> take into account the amount by which the stack address was randomized.
> In other words, the stack gets less than RLIMIT_STACK space.

Is this entirely accurate? The top of the task would be task_size, but
this code is using task_size / 6 * 5 as the bottom of stack / top of
mmap gap_max. Is there a reason for this?

>
> Ensure that the gap between the stack and mmap_base always takes stack
> randomization into account.
>
> From Daniel Micay's linux-hardened tree.
>
> Reported-by: Florian Weimer <fweimer@...hat.com>
> Signed-off-by: Daniel Micay <danielmicay@...il.com>
> Signed-off-by: Rik van Riel <riel@...hat.com>
> ---
>  arch/x86/mm/mmap.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 19ad095b41df..8c7ba1adb27b 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -95,13 +95,18 @@ unsigned long arch_mmap_rnd(void)
>  static unsigned long mmap_base(unsigned long rnd, unsigned long task_size)
>  {
>         unsigned long gap = rlimit(RLIMIT_STACK);
> +       unsigned long pad = stack_maxrandom_size(task_size);
>         unsigned long gap_min, gap_max;
>
> +       /* Values close to RLIM_INFINITY can overflow. */
> +       if (gap + pad > gap)
> +               gap += pad;
> +
>         /*
>          * Top of mmap area (just below the process stack).
>          * Leave an at least ~128 MB hole with possible stack randomization.
>          */
> -       gap_min = SIZE_128M + stack_maxrandom_size(task_size);
> +       gap_min = SIZE_128M;
>         gap_max = (task_size / 6) * 5;
>
>         if (gap < gap_min)

-Kees

-- 
Kees Cook
Pixel Security

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.