Re: [PATCH] x86, kaslr: randomize module base load address

Andrew Morton
2014-02-21T20:37:00Z

On Fri, 21 Feb 2014 12:21:10 -0800 Kees Cook <keescook@chromium.org> wrote:

> From: Andy Honig <ahonig@google.com>
>
> Randomize the load address of modules in the kernel to make kASLR
> effective for modules. Modules can only be loaded within a particular
> range of virtual address space. This patch adds 10 bits of entropy to
> the load address by adding 1-1024 * PAGE_SIZE to the beginning range
> where modules are loaded.
>
> Example kASLR boot without this change, with a single module loaded:
> ---[ Modules ]---
> 0xffffffffc0000000-0xffffffffc0001000 4K ro GLB x pte
> 0xffffffffc0001000-0xffffffffc0002000 4K ro GLB NX pte
> 0xffffffffc0002000-0xffffffffc0004000 8K RW GLB NX pte
> 0xffffffffc0004000-0xffffffffc0200000 2032K pte
> 0xffffffffc0200000-0xffffffffff000000 1006M pmd
> ---[ End Modules ]---
>
> Example kASLR boot after this change, same module loaded:
> ---[ Modules ]---
> 0xffffffffc0000000-0xffffffffc0200000 2M pmd
> 0xffffffffc0200000-0xffffffffc03bf000 1788K pte
> 0xffffffffc03bf000-0xffffffffc03c0000 4K ro GLB x pte
> 0xffffffffc03c0000-0xffffffffc03c1000 4K ro GLB NX pte
> 0xffffffffc03c1000-0xffffffffc03c3000 8K RW GLB NX pte
> 0xffffffffc03c3000-0xffffffffc0400000 244K pte
> 0xffffffffc0400000-0xffffffffff000000 1004M pmd
> ---[ End Modules ]---
>
> ...
>
> --- a/arch/x86/kernel/module.c
> +++ b/arch/x86/kernel/module.c
> @@ -28,6 +28,7 @@
> #include <linux/mm.h>
> #include <linux/gfp.h>
> #include <linux/jump_label.h>
> +#include <linux/random.h>
>
> #include <asm/page.h>
> #include <asm/pgtable.h>
> @@ -43,13 +44,49 @@ do { \
> } while (0)
> #endif
>
> +#ifdef CONFIG_RANDOMIZE_BASE
> +static unsigned long module_load_offset;
> +static int randomize_modules = 1;

It's pretty common for people to later come back and say "hey I want to
set the default in Kconfig". Perhaps we should do that from day 1.

This implies that parse_nokaslr() will need to be renamed and taught to
handle 0->1 changing.

> +static int __init parse_nokaslr(char *p)
> +{
> + randomize_modules = 0;
> + return 0;
> +}
> +early_param("nokaslr", parse_nokaslr);

Documentation/kernel-parameters.txt, please. This isn't hard :(

> +static unsigned long int get_module_load_offset(void)
> +{
> + if (randomize_modules) {
> + mutex_lock(&module_mutex);
> + /*
> + * Calculate the module_load_offset the first time this
> + * code is called. Once calculated it stays the same until
> + * reboot.
> + */
> + if (module_load_offset == 0)
> + module_load_offset =
> + (get_random_int() % 1024 + 1) * PAGE_SIZE;
> + mutex_unlock(&module_mutex);
> + }
> + return module_load_offset;
> +}

This seems unnecessarily complex and inefficient. We only set
module_load_offset a single time, so why not do it that way?
Mark it __init, run it during initcalls then throw it away.

> +#else
> +static unsigned long int get_module_load_offset(void)
> +{
> + return 0;
> +}
> +#endif
> +
> void *module_alloc(unsigned long size)
> {
> if (PAGE_ALIGN(size) > MODULES_LEN)
> return NULL;
> - return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END,
> - GFP_KERNEL | __GFP_HIGHMEM, PAGE_KERNEL_EXEC,
> - NUMA_NO_NODE, __builtin_return_address(0));
> + return __vmalloc_node_range(size, 1,
> + MODULES_VADDR + get_module_load_offset(),
> + MODULES_END, GFP_KERNEL | __GFP_HIGHMEM,
> + PAGE_KERNEL_EXEC, NUMA_NO_NODE,
> + __builtin_return_address(0));
> }
>
> #ifdef CONFIG_X86_32