Re: [kernel-hardening] [PATCH 3/6] x86/mmap: properly account for stack randomization in mmap_base

[Kees Cook]

On Fri, Jun 2, 2017 at 8:20 AM,  <riel@xxxxxxxxxx> wrote:
> From: Rik van Riel <riel@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Daniel Micay <danielmicay@xxxxxxxxx>
> Signed-off-by: Rik van Riel <riel@xxxxxxxxxx>
> ---
>  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