Re: [PATCH v6 2/3] x86/mm: Implement ASLR for kernel memory sections (x86_64)

[Thomas Garnier]

On Fri, Jun 17, 2016 at 3:26 AM, Ingo Molnar  wrote:
>
> * Kees Cook  wrote:
>
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -1993,6 +1993,23 @@ config PHYSICAL_ALIGN
>>
>> Don't change this unless you know what you are doing.
>>
>> +config RANDOMIZE_MEMORY
>> + bool "Randomize the kernel memory sections"
>> + depends on X86_64
>> + depends on RANDOMIZE_BASE
>> + default RANDOMIZE_BASE
>> + ---help---
>> +Randomizes the base virtual address of kernel memory sections
>> +(physical memory mapping, vmalloc & vmemmap). This security feature
>> +makes exploits relying on predictable memory locations less 
>> reliable.
>> +
>> +The order of allocations remains unchanged. Entropy is generated in
>> +the same way as RANDOMIZE_BASE. Current implementation in the 
>> optimal
>> +configuration have in average 30,000 different possible virtual
>> +addresses for each memory section.
>> +
>> +If unsure, say N.
>
> So this is really poor naming!
>
> The user, in first approximation, will see something like this:
>
>   Randomize the kernel memory sections (RANDOMIZE_MEMORY) [Y/n/?] (NEW)
>
> ... and could naively conclude that this feature will randomize memory 
> contents.
>
> Furthermore, there's no apparent connection to another, related kernel 
> feature:
>
>   Randomize the address of the kernel image (KASLR) (RANDOMIZE_BASE) [Y/n/?]
>
> A better naming would be something like:
>
>  Randomize other static kernel memory addresses (RANDOMIZE_ALL) [Y/n/?] 
> (NEW)
>
> (assuming it's truly 'all')
>
> But ... I don't see it explained anywhere why a user, once he expressed 
> interest
> in randomization would even want to _not_ randomize as much as possible. I.e. 
> why
> does this new option exist at all, shouldn't we just gradually extend
> randomization to more memory areas and control it via CONFIG_RANDOMIZE_BASE?
>
> Btw., CONFIG_RANDOMIZE_BASE is probably a misnomer, and after these patches it
> will be more of a misnomer - so we should rename it to something better. For
> example CONFIG_KASLR would have the advantage of being obviously named at a
> glance, to anyone who knows what 'ASLR' means. To those who don't know the 
> short
> description will tell it:
>
>   Kernel address space layout randomization (KASLR) [Y/n/?]
>
> This would also fit the kernel internal naming of kaslr.c/etc.
>
> What do you think?
>
>> +++ b/arch/x86/mm/kaslr.c
>> @@ -0,0 +1,167 @@
>> +/*
>> + * This file implements KASLR memory randomization for x86_64. It randomizes
>> + * the virtual address space of kernel memory sections (physical memory
>> + * mapping, vmalloc & vmemmap) for x86_64. This security feature mitigates
>> + * exploits relying on predictable kernel addresses.
>> + *
>> + * Entropy is generated using the KASLR early boot functions now shared in
>> + * the lib directory (originally written by Kees Cook). Randomization is
>> + * done on PGD & PUD page table levels to increase possible addresses. The
>> + * physical memory mapping code was adapted to support PUD level virtual
>> + * addresses. This implementation on the best configuration provides 30,000
>> + * possible virtual addresses in average for each memory section.  An
>> + * additional low memory page is used to ensure each CPU can start with a
>> + * PGD aligned virtual address (for realmode).
>> + *
>> + * The order of each memory section is not changed. The feature looks at
>> + * the available space for the sections based on different configuration
>> + * options and randomizes the base and space between each. The size of the
>> + * physical memory mapping is the available physical memory.
>> + *
>> + */
>
> (Stray newline.)
>
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "mm_internal.h"
>
> How many of those include lines are truly unnecessary, or did this just got 
> copied
> from another file?
>

No that's correct. I will clean that up.

>> +/*
>> + * Memory base and end randomization is based on different configurations.
>> + * We want as much space as possible to increase entropy available.
>> + */
>> +static const unsigned long memory_rand_start = __PAGE_OFFSET_BASE;
>> +
>> +#if defined(CONFIG_KASAN)
>> +static const unsigned long memory_rand_end = KASAN_SHADOW_START;
>> +#elif defined(CONFIG_X86_ESPFIX64)
>> +static const unsigned long memory_rand_end = ESPFIX_BASE_ADDR;
>> +#elif defined(CONFIG_EFI)
>> +static const unsigned long memory_rand_end = EFI_VA_START;
>> +#else
>> +static const unsigned long memory_rand_end = __START_KERNEL_map;
>> +#endif
>
> It's unclear to me from the naming and the description whether these are 
> virtual
> or physical memory ranges. Nor is it explained why this laundry list of 

Re: [PATCH v6 2/3] x86/mm: Implement ASLR for kernel memory sections (x86_64)

[Thomas Garnier]

On Fri, Jun 17, 2016 at 3:26 AM, Ingo Molnar  wrote:
>
> * Kees Cook  wrote:
>
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -1993,6 +1993,23 @@ config PHYSICAL_ALIGN
>>
>> Don't change this unless you know what you are doing.
>>
>> +config RANDOMIZE_MEMORY
>> + bool "Randomize the kernel memory sections"
>> + depends on X86_64
>> + depends on RANDOMIZE_BASE
>> + default RANDOMIZE_BASE
>> + ---help---
>> +Randomizes the base virtual address of kernel memory sections
>> +(physical memory mapping, vmalloc & vmemmap). This security feature
>> +makes exploits relying on predictable memory locations less 
>> reliable.
>> +
>> +The order of allocations remains unchanged. Entropy is generated in
>> +the same way as RANDOMIZE_BASE. Current implementation in the 
>> optimal
>> +configuration have in average 30,000 different possible virtual
>> +addresses for each memory section.
>> +
>> +If unsure, say N.
>
> So this is really poor naming!
>
> The user, in first approximation, will see something like this:
>
>   Randomize the kernel memory sections (RANDOMIZE_MEMORY) [Y/n/?] (NEW)
>
> ... and could naively conclude that this feature will randomize memory 
> contents.
>
> Furthermore, there's no apparent connection to another, related kernel 
> feature:
>
>   Randomize the address of the kernel image (KASLR) (RANDOMIZE_BASE) [Y/n/?]
>
> A better naming would be something like:
>
>  Randomize other static kernel memory addresses (RANDOMIZE_ALL) [Y/n/?] 
> (NEW)
>
> (assuming it's truly 'all')
>
> But ... I don't see it explained anywhere why a user, once he expressed 
> interest
> in randomization would even want to _not_ randomize as much as possible. I.e. 
> why
> does this new option exist at all, shouldn't we just gradually extend
> randomization to more memory areas and control it via CONFIG_RANDOMIZE_BASE?
>
> Btw., CONFIG_RANDOMIZE_BASE is probably a misnomer, and after these patches it
> will be more of a misnomer - so we should rename it to something better. For
> example CONFIG_KASLR would have the advantage of being obviously named at a
> glance, to anyone who knows what 'ASLR' means. To those who don't know the 
> short
> description will tell it:
>
>   Kernel address space layout randomization (KASLR) [Y/n/?]
>
> This would also fit the kernel internal naming of kaslr.c/etc.
>
> What do you think?
>
>> +++ b/arch/x86/mm/kaslr.c
>> @@ -0,0 +1,167 @@
>> +/*
>> + * This file implements KASLR memory randomization for x86_64. It randomizes
>> + * the virtual address space of kernel memory sections (physical memory
>> + * mapping, vmalloc & vmemmap) for x86_64. This security feature mitigates
>> + * exploits relying on predictable kernel addresses.
>> + *
>> + * Entropy is generated using the KASLR early boot functions now shared in
>> + * the lib directory (originally written by Kees Cook). Randomization is
>> + * done on PGD & PUD page table levels to increase possible addresses. The
>> + * physical memory mapping code was adapted to support PUD level virtual
>> + * addresses. This implementation on the best configuration provides 30,000
>> + * possible virtual addresses in average for each memory section.  An
>> + * additional low memory page is used to ensure each CPU can start with a
>> + * PGD aligned virtual address (for realmode).
>> + *
>> + * The order of each memory section is not changed. The feature looks at
>> + * the available space for the sections based on different configuration
>> + * options and randomizes the base and space between each. The size of the
>> + * physical memory mapping is the available physical memory.
>> + *
>> + */
>
> (Stray newline.)
>
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "mm_internal.h"
>
> How many of those include lines are truly unnecessary, or did this just got 
> copied
> from another file?
>

No that's correct. I will clean that up.

>> +/*
>> + * Memory base and end randomization is based on different configurations.
>> + * We want as much space as possible to increase entropy available.
>> + */
>> +static const unsigned long memory_rand_start = __PAGE_OFFSET_BASE;
>> +
>> +#if defined(CONFIG_KASAN)
>> +static const unsigned long memory_rand_end = KASAN_SHADOW_START;
>> +#elif defined(CONFIG_X86_ESPFIX64)
>> +static const unsigned long memory_rand_end = ESPFIX_BASE_ADDR;
>> +#elif defined(CONFIG_EFI)
>> +static const unsigned long memory_rand_end = EFI_VA_START;
>> +#else
>> +static const unsigned long memory_rand_end = __START_KERNEL_map;
>> +#endif
>
> It's unclear to me from the naming and the description whether these are 
> virtual
> or physical memory ranges. Nor is it explained why this laundry list of kernel
> options influences the 

Re: [PATCH v6 2/3] x86/mm: Implement ASLR for kernel memory sections (x86_64)

[Kees Cook]

Thanks for the review! I'll let Thomas address the feedback, though
I've got some thoughts below on naming.

On Fri, Jun 17, 2016 at 3:26 AM, Ingo Molnar  wrote:
>
> * Kees Cook  wrote:
>
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -1993,6 +1993,23 @@ config PHYSICAL_ALIGN
>>
>> Don't change this unless you know what you are doing.
>>
>> +config RANDOMIZE_MEMORY
>> + bool "Randomize the kernel memory sections"
>> + depends on X86_64
>> + depends on RANDOMIZE_BASE
>> + default RANDOMIZE_BASE
>> + ---help---
>> +Randomizes the base virtual address of kernel memory sections
>> +(physical memory mapping, vmalloc & vmemmap). This security feature
>> +makes exploits relying on predictable memory locations less 
>> reliable.
>> +
>> +The order of allocations remains unchanged. Entropy is generated in
>> +the same way as RANDOMIZE_BASE. Current implementation in the 
>> optimal
>> +configuration have in average 30,000 different possible virtual
>> +addresses for each memory section.
>> +
>> +If unsure, say N.
>
> So this is really poor naming!

I think "RANDOMIZE" should probably be changed, yes, though I have
some caveats...

> The user, in first approximation, will see something like this:
>
>   Randomize the kernel memory sections (RANDOMIZE_MEMORY) [Y/n/?] (NEW)
>
> ... and could naively conclude that this feature will randomize memory 
> contents.
>
> Furthermore, there's no apparent connection to another, related kernel 
> feature:
>
>   Randomize the address of the kernel image (KASLR) (RANDOMIZE_BASE) [Y/n/?]
>
> A better naming would be something like:
>
>  Randomize other static kernel memory addresses (RANDOMIZE_ALL) [Y/n/?] 
> (NEW)
>
> (assuming it's truly 'all')

There's always more to be identified and added. A few thoughts:

"Address Space Layout Randomization" is a recognizable name, though I
tried to avoid it in the descriptive texts because
CONFIG_RANDOMIZE_BASE does not randomize "everything" and it doesn't
randomize link order, etc, it only provides a randomized base address
to the kernel text. Now, this is in line with all the userspace ASLR:
each one of stack, mmap, brk, and text have their ASLR done via base
address offsets. ASLR is a technique, and is frequently confused with
"coverage", which I have been trying to fix. i.e. Kernel ASLR of text
base address is one piece of coverage, but it leaves other things not
yet ASLRed.

So, now that we have something else besides base text address ASLR,
I'm happy to create a configs that carry the name "KASLR".

> But ... I don't see it explained anywhere why a user, once he expressed 
> interest
> in randomization would even want to _not_ randomize as much as possible. I.e. 
> why
> does this new option exist at all, shouldn't we just gradually extend
> randomization to more memory areas and control it via CONFIG_RANDOMIZE_BASE?

I think it's important to maintain a higher granularity of CONFIG
options since the maturity of each given feature will vary from arch
to arch. Also, it becomes hard to compare, for example, an x86 .config
with an arm64 .config to see which features are enabled. If
CONFIG_RANDOMIZE_MEMORY landed, it'd be easy to see that it is missing
on arm64 from looking at .config files.

Now, that said, a lot of people have wanted a "make this build as
secure as possible" CONFIG, so it could be nice to have a CONFIG_KASLR
that selects each of the available sub-configs.

> Btw., CONFIG_RANDOMIZE_BASE is probably a misnomer, and after these patches it
> will be more of a misnomer - so we should rename it to something better. For
> example CONFIG_KASLR would have the advantage of being obviously named at a
> glance, to anyone who knows what 'ASLR' means. To those who don't know the 
> short
> description will tell it:
>
>   Kernel address space layout randomization (KASLR) [Y/n/?]
>
> This would also fit the kernel internal naming of kaslr.c/etc.
>
> What do you think?

How about something like this:

CONFIG_KASLR
depends on HAVE_ARCH_KASLR_TEXT || HAVE_ARCH_KASLR_MEMORY
CONFIG_KASLR_TEXT
depends on CONFIG_KASLR && HAVE_ARCH_KASLR_TEXT
default CONFIG_KASLR
CONFIG_KASLR_MEMORY
depends on CONFIG_KASLR && HAVE_ARCH_KASLR_MEMORY
default CONFIG_KASLR

And then we can select HAVE_ARCH_KASLR_TEXT for x86, arm64, and MIPS,
and when this lands, x86 would gain HAVE_ARCH_KASLR_MEMORY.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

Re: [PATCH v6 2/3] x86/mm: Implement ASLR for kernel memory sections (x86_64)

[Kees Cook]

Thanks for the review! I'll let Thomas address the feedback, though
I've got some thoughts below on naming.

On Fri, Jun 17, 2016 at 3:26 AM, Ingo Molnar  wrote:
>
> * Kees Cook  wrote:
>
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -1993,6 +1993,23 @@ config PHYSICAL_ALIGN
>>
>> Don't change this unless you know what you are doing.
>>
>> +config RANDOMIZE_MEMORY
>> + bool "Randomize the kernel memory sections"
>> + depends on X86_64
>> + depends on RANDOMIZE_BASE
>> + default RANDOMIZE_BASE
>> + ---help---
>> +Randomizes the base virtual address of kernel memory sections
>> +(physical memory mapping, vmalloc & vmemmap). This security feature
>> +makes exploits relying on predictable memory locations less 
>> reliable.
>> +
>> +The order of allocations remains unchanged. Entropy is generated in
>> +the same way as RANDOMIZE_BASE. Current implementation in the 
>> optimal
>> +configuration have in average 30,000 different possible virtual
>> +addresses for each memory section.
>> +
>> +If unsure, say N.
>
> So this is really poor naming!

I think "RANDOMIZE" should probably be changed, yes, though I have
some caveats...

> The user, in first approximation, will see something like this:
>
>   Randomize the kernel memory sections (RANDOMIZE_MEMORY) [Y/n/?] (NEW)
>
> ... and could naively conclude that this feature will randomize memory 
> contents.
>
> Furthermore, there's no apparent connection to another, related kernel 
> feature:
>
>   Randomize the address of the kernel image (KASLR) (RANDOMIZE_BASE) [Y/n/?]
>
> A better naming would be something like:
>
>  Randomize other static kernel memory addresses (RANDOMIZE_ALL) [Y/n/?] 
> (NEW)
>
> (assuming it's truly 'all')

There's always more to be identified and added. A few thoughts:

"Address Space Layout Randomization" is a recognizable name, though I
tried to avoid it in the descriptive texts because
CONFIG_RANDOMIZE_BASE does not randomize "everything" and it doesn't
randomize link order, etc, it only provides a randomized base address
to the kernel text. Now, this is in line with all the userspace ASLR:
each one of stack, mmap, brk, and text have their ASLR done via base
address offsets. ASLR is a technique, and is frequently confused with
"coverage", which I have been trying to fix. i.e. Kernel ASLR of text
base address is one piece of coverage, but it leaves other things not
yet ASLRed.

So, now that we have something else besides base text address ASLR,
I'm happy to create a configs that carry the name "KASLR".

> But ... I don't see it explained anywhere why a user, once he expressed 
> interest
> in randomization would even want to _not_ randomize as much as possible. I.e. 
> why
> does this new option exist at all, shouldn't we just gradually extend
> randomization to more memory areas and control it via CONFIG_RANDOMIZE_BASE?

I think it's important to maintain a higher granularity of CONFIG
options since the maturity of each given feature will vary from arch
to arch. Also, it becomes hard to compare, for example, an x86 .config
with an arm64 .config to see which features are enabled. If
CONFIG_RANDOMIZE_MEMORY landed, it'd be easy to see that it is missing
on arm64 from looking at .config files.

Now, that said, a lot of people have wanted a "make this build as
secure as possible" CONFIG, so it could be nice to have a CONFIG_KASLR
that selects each of the available sub-configs.

> Btw., CONFIG_RANDOMIZE_BASE is probably a misnomer, and after these patches it
> will be more of a misnomer - so we should rename it to something better. For
> example CONFIG_KASLR would have the advantage of being obviously named at a
> glance, to anyone who knows what 'ASLR' means. To those who don't know the 
> short
> description will tell it:
>
>   Kernel address space layout randomization (KASLR) [Y/n/?]
>
> This would also fit the kernel internal naming of kaslr.c/etc.
>
> What do you think?

How about something like this:

CONFIG_KASLR
depends on HAVE_ARCH_KASLR_TEXT || HAVE_ARCH_KASLR_MEMORY
CONFIG_KASLR_TEXT
depends on CONFIG_KASLR && HAVE_ARCH_KASLR_TEXT
default CONFIG_KASLR
CONFIG_KASLR_MEMORY
depends on CONFIG_KASLR && HAVE_ARCH_KASLR_MEMORY
default CONFIG_KASLR

And then we can select HAVE_ARCH_KASLR_TEXT for x86, arm64, and MIPS,
and when this lands, x86 would gain HAVE_ARCH_KASLR_MEMORY.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

Re: [PATCH v6 2/3] x86/mm: Implement ASLR for kernel memory sections (x86_64)

[Ingo Molnar]

* Kees Cook  wrote:

> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1993,6 +1993,23 @@ config PHYSICAL_ALIGN
>  
> Don't change this unless you know what you are doing.
>  
> +config RANDOMIZE_MEMORY
> + bool "Randomize the kernel memory sections"
> + depends on X86_64
> + depends on RANDOMIZE_BASE
> + default RANDOMIZE_BASE
> + ---help---
> +Randomizes the base virtual address of kernel memory sections
> +(physical memory mapping, vmalloc & vmemmap). This security feature
> +makes exploits relying on predictable memory locations less reliable.
> +
> +The order of allocations remains unchanged. Entropy is generated in
> +the same way as RANDOMIZE_BASE. Current implementation in the optimal
> +configuration have in average 30,000 different possible virtual
> +addresses for each memory section.
> +
> +If unsure, say N.

So this is really poor naming!

The user, in first approximation, will see something like this:

  Randomize the kernel memory sections (RANDOMIZE_MEMORY) [Y/n/?] (NEW) 

... and could naively conclude that this feature will randomize memory contents.

Furthermore, there's no apparent connection to another, related kernel feature:

  Randomize the address of the kernel image (KASLR) (RANDOMIZE_BASE) [Y/n/?]

A better naming would be something like:

 Randomize other static kernel memory addresses (RANDOMIZE_ALL) [Y/n/?] 
(NEW) 

(assuming it's truly 'all')

But ... I don't see it explained anywhere why a user, once he expressed 
interest 
in randomization would even want to _not_ randomize as much as possible. I.e. 
why 
does this new option exist at all, shouldn't we just gradually extend 
randomization to more memory areas and control it via CONFIG_RANDOMIZE_BASE?

Btw., CONFIG_RANDOMIZE_BASE is probably a misnomer, and after these patches it 
will be more of a misnomer - so we should rename it to something better. For 
example CONFIG_KASLR would have the advantage of being obviously named at a 
glance, to anyone who knows what 'ASLR' means. To those who don't know the 
short 
description will tell it:

  Kernel address space layout randomization (KASLR) [Y/n/?]

This would also fit the kernel internal naming of kaslr.c/etc.

What do you think?

> +++ b/arch/x86/mm/kaslr.c
> @@ -0,0 +1,167 @@
> +/*
> + * This file implements KASLR memory randomization for x86_64. It randomizes
> + * the virtual address space of kernel memory sections (physical memory
> + * mapping, vmalloc & vmemmap) for x86_64. This security feature mitigates
> + * exploits relying on predictable kernel addresses.
> + *
> + * Entropy is generated using the KASLR early boot functions now shared in
> + * the lib directory (originally written by Kees Cook). Randomization is
> + * done on PGD & PUD page table levels to increase possible addresses. The
> + * physical memory mapping code was adapted to support PUD level virtual
> + * addresses. This implementation on the best configuration provides 30,000
> + * possible virtual addresses in average for each memory section.  An
> + * additional low memory page is used to ensure each CPU can start with a
> + * PGD aligned virtual address (for realmode).
> + *
> + * The order of each memory section is not changed. The feature looks at
> + * the available space for the sections based on different configuration
> + * options and randomizes the base and space between each. The size of the
> + * physical memory mapping is the available physical memory.
> + *
> + */

(Stray newline.)

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "mm_internal.h"

How many of those include lines are truly unnecessary, or did this just got 
copied 
from another file?

> +/*
> + * Memory base and end randomization is based on different configurations.
> + * We want as much space as possible to increase entropy available.
> + */
> +static const unsigned long memory_rand_start = __PAGE_OFFSET_BASE;
> +
> +#if defined(CONFIG_KASAN)
> +static const unsigned long memory_rand_end = KASAN_SHADOW_START;
> +#elif defined(CONFIG_X86_ESPFIX64)
> +static const unsigned long memory_rand_end = ESPFIX_BASE_ADDR;
> +#elif defined(CONFIG_EFI)
> +static const unsigned long memory_rand_end = EFI_VA_START;
> +#else
> +static const unsigned long memory_rand_end = __START_KERNEL_map;
> +#endif

It's unclear to me from the naming and the description whether these are 
virtual 
or physical memory ranges. Nor is it explained why this laundry list of kernel 
options influences the randomization range - and it's not explained how new 
kernel 
features modifying the virtual memory layout should plug into this mechanism.

Could we please get a bit more verbose introduction into why all of this is 
done?

> +/* Default values */

Re: [PATCH v6 2/3] x86/mm: Implement ASLR for kernel memory sections (x86_64)

[Ingo Molnar]

* Kees Cook  wrote:

> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1993,6 +1993,23 @@ config PHYSICAL_ALIGN
>  
> Don't change this unless you know what you are doing.
>  
> +config RANDOMIZE_MEMORY
> + bool "Randomize the kernel memory sections"
> + depends on X86_64
> + depends on RANDOMIZE_BASE
> + default RANDOMIZE_BASE
> + ---help---
> +Randomizes the base virtual address of kernel memory sections
> +(physical memory mapping, vmalloc & vmemmap). This security feature
> +makes exploits relying on predictable memory locations less reliable.
> +
> +The order of allocations remains unchanged. Entropy is generated in
> +the same way as RANDOMIZE_BASE. Current implementation in the optimal
> +configuration have in average 30,000 different possible virtual
> +addresses for each memory section.
> +
> +If unsure, say N.

So this is really poor naming!

The user, in first approximation, will see something like this:

  Randomize the kernel memory sections (RANDOMIZE_MEMORY) [Y/n/?] (NEW) 

... and could naively conclude that this feature will randomize memory contents.

Furthermore, there's no apparent connection to another, related kernel feature:

  Randomize the address of the kernel image (KASLR) (RANDOMIZE_BASE) [Y/n/?]

A better naming would be something like:

 Randomize other static kernel memory addresses (RANDOMIZE_ALL) [Y/n/?] 
(NEW) 

(assuming it's truly 'all')

But ... I don't see it explained anywhere why a user, once he expressed 
interest 
in randomization would even want to _not_ randomize as much as possible. I.e. 
why 
does this new option exist at all, shouldn't we just gradually extend 
randomization to more memory areas and control it via CONFIG_RANDOMIZE_BASE?

Btw., CONFIG_RANDOMIZE_BASE is probably a misnomer, and after these patches it 
will be more of a misnomer - so we should rename it to something better. For 
example CONFIG_KASLR would have the advantage of being obviously named at a 
glance, to anyone who knows what 'ASLR' means. To those who don't know the 
short 
description will tell it:

  Kernel address space layout randomization (KASLR) [Y/n/?]

This would also fit the kernel internal naming of kaslr.c/etc.

What do you think?

> +++ b/arch/x86/mm/kaslr.c
> @@ -0,0 +1,167 @@
> +/*
> + * This file implements KASLR memory randomization for x86_64. It randomizes
> + * the virtual address space of kernel memory sections (physical memory
> + * mapping, vmalloc & vmemmap) for x86_64. This security feature mitigates
> + * exploits relying on predictable kernel addresses.
> + *
> + * Entropy is generated using the KASLR early boot functions now shared in
> + * the lib directory (originally written by Kees Cook). Randomization is
> + * done on PGD & PUD page table levels to increase possible addresses. The
> + * physical memory mapping code was adapted to support PUD level virtual
> + * addresses. This implementation on the best configuration provides 30,000
> + * possible virtual addresses in average for each memory section.  An
> + * additional low memory page is used to ensure each CPU can start with a
> + * PGD aligned virtual address (for realmode).
> + *
> + * The order of each memory section is not changed. The feature looks at
> + * the available space for the sections based on different configuration
> + * options and randomizes the base and space between each. The size of the
> + * physical memory mapping is the available physical memory.
> + *
> + */

(Stray newline.)

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "mm_internal.h"

How many of those include lines are truly unnecessary, or did this just got 
copied 
from another file?

> +/*
> + * Memory base and end randomization is based on different configurations.
> + * We want as much space as possible to increase entropy available.
> + */
> +static const unsigned long memory_rand_start = __PAGE_OFFSET_BASE;
> +
> +#if defined(CONFIG_KASAN)
> +static const unsigned long memory_rand_end = KASAN_SHADOW_START;
> +#elif defined(CONFIG_X86_ESPFIX64)
> +static const unsigned long memory_rand_end = ESPFIX_BASE_ADDR;
> +#elif defined(CONFIG_EFI)
> +static const unsigned long memory_rand_end = EFI_VA_START;
> +#else
> +static const unsigned long memory_rand_end = __START_KERNEL_map;
> +#endif

It's unclear to me from the naming and the description whether these are 
virtual 
or physical memory ranges. Nor is it explained why this laundry list of kernel 
options influences the randomization range - and it's not explained how new 
kernel 
features modifying the virtual memory layout should plug into this mechanism.

Could we please get a bit more verbose introduction into why all of this is 
done?

> +/* Default values */
> +unsigned long