Re: [PATCH] x86/boot/KASLR: Extend movable_node option for KASLR

[Dou Liyang <douly.fnst@cn.fujitsu.com>]

Hi Kees,

At 08/04/2017 06:40 AM, Kees Cook wrote:
> On Thu, Aug 3, 2017 at 5:17 AM, Dou Liyang <douly.fnst@cn.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@cn.fujitsu.com>
>> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
>
> This seems reasonable to me. Thanks for the fix!
>

It's my pleasure!

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

Thanks for reviewing.

Thanks,
	dou.
>> ---
>>  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
>>
>>
>>
>
>
>