Message-ID: <CAGXu5jLAY1MDF-Xz2kS9C-KVXfk3eLKteEQP+PgQMD6cHY6_Rg@mail.gmail.com>
Date:   Thu, 3 Aug 2017 15:40:35 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Dou Liyang <douly.fnst@...fujitsu.com>, Baoquan He <bhe@...hat.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>, fanc.fnst@...fujitsu.com,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Dave Young <dyoung@...hat.com>,
        Arnd Bergmann <arnd@...db.de>,
        Dave Jiang <dave.jiang@...el.com>, indou.takao@...fujitsu.com,
        izumi.taku@...fujitsu.com
Subject: Re: [PATCH] x86/boot/KASLR: Extend movable_node option for KASLR

On Thu, Aug 3, 2017 at 5:17 AM, Dou Liyang <douly.fnst@...fujitsu.com> wrote:
> movable_node is a boot-time switch to make hot-pluggable memory
> NUMA nodes to be movable. This option is based on an assumption
> that any node which the kernel resides in is defined as
> un-hotpluggable. Linux can allocates memory near the kernel image
> to try the best to keep the kernel away from hotpluggable memory
> in the same NUMA node. So other nodes can be movable.
>
> But, KASLR doesn't know which node is un-hotpluggable, the all
> hotpluggable memory ranges is recorded in ACPI SRAT table, SRAT
> is not parsed. So, KASLR may randomize the kernel in a movable
> node which will be immovable.
>
> Extend movable_node option to restrict kernel to be randomized in
> immovable nodes by adding a parameter. this parameter sets up
> the boundaries between the movable nodes and immovable nodes.
>
> Reported-by: Chao Fan <fanc.fnst@...fujitsu.com>
> Signed-off-by: Dou Liyang <douly.fnst@...fujitsu.com>

This seems reasonable to me. Thanks for the fix!

Reviewed-by: Kees Cook <keescook@...omium.org>

> ---
>  Documentation/admin-guide/kernel-parameters.txt | 11 +++++++++--
>  arch/x86/boot/compressed/kaslr.c                | 19 ++++++++++++++++---
>  2 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index d9c171c..44c7e33 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2305,7 +2305,8 @@
>         mousedev.yres=  [MOUSE] Vertical screen resolution, used for devices
>                         reporting absolute coordinates, such as tablets
>
> -       movablecore=nn[KMG]     [KNL,X86,IA-64,PPC] This parameter
> +       movablecore=nn[KMG]
> +                       [KNL,X86,IA-64,PPC] This parameter
>                         is similar to kernelcore except it specifies the
>                         amount of memory used for migratable allocations.
>                         If both kernelcore and movablecore is specified,
> @@ -2315,12 +2316,18 @@
>                         that the amount of memory usable for all allocations
>                         is not too small.
>
> -       movable_node    [KNL] Boot-time switch to make hotplugable memory
> +       movable_node    [KNL] Boot-time switch to make hot-pluggable memory
>                         NUMA nodes to be movable. This means that the memory
>                         of such nodes will be usable only for movable
>                         allocations which rules out almost all kernel
>                         allocations. Use with caution!
>
> +       movable_node=nn[KMG]
> +                       [KNL] Extend movable_node to work well with KASLR. This
> +                       parameter is the boundaries between the movable nodes
> +                       and immovable nodes, the memory which exceeds it will
> +                       be regarded as hot-pluggable.
> +
>         MTD_Partition=  [MTD]
>                         Format: <name>,<region-number>,<size>,<offset>
>
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 91f27ab..7e2351b 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -89,7 +89,10 @@ struct mem_vector {
>  static bool memmap_too_large;
>
>
> -/* Store memory limit specified by "mem=nn[KMG]" or "memmap=nn[KMG]" */
> +/*
> + * Store memory limit specified by the following situations:
> + * "mem=nn[KMG]" or "memmap=nn[KMG]" or "movable_node=nn[KMG]"
> + */
>  unsigned long long mem_limit = ULLONG_MAX;
>
>
> @@ -212,7 +215,8 @@ static int handle_mem_memmap(void)
>         char *param, *val;
>         u64 mem_size;
>
> -       if (!strstr(args, "memmap=") && !strstr(args, "mem="))
> +       if (!strstr(args, "memmap=") && !strstr(args, "mem=") &&
> +               !strstr(args, "movable_node="))
>                 return 0;
>
>         tmp_cmdline = malloc(len + 1);
> @@ -247,7 +251,16 @@ static int handle_mem_memmap(void)
>                                 free(tmp_cmdline);
>                                 return -EINVAL;
>                         }
> -                       mem_limit = mem_size;
> +                       mem_limit = mem_limit > mem_size ? mem_size : mem_limit;
> +               } else if (!strcmp(param, "movable_node")) {
> +                       char *p = val;
> +
> +                       mem_size = memparse(p, &p);
> +                       if (mem_size == 0) {
> +                               free(tmp_cmdline);
> +                               return -EINVAL;
> +                       }
> +                       mem_limit = mem_limit > mem_size ? mem_size : mem_limit;
>                 }
>         }
>
> --
> 2.5.5
>
>
>



-- 
Kees Cook
Pixel Security

Powered by blists - more mailing lists