| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 21 Feb 2014 13:05:08 -0800
From: Kees Cook <keescook@...omium.org>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>,
"x86@...nel.org" <x86@...nel.org>,
Jianguo Wu <wujianguo@...wei.com>,
Andy Honig <ahonig@...gle.com>,
David Rientjes <rientjes@...gle.com>
Subject: Re: [PATCH] x86, kaslr: randomize module base load address
On Fri, Feb 21, 2014 at 12:36 PM, Andrew Morton
<akpm@...ux-foundation.org> wrote:
> On Fri, 21 Feb 2014 12:21:10 -0800 Kees Cook <keescook@...omium.org> wrote:
>
>> From: Andy Honig <ahonig@...gle.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.
I've been slapped down for adding more config options in the past, and
I think it's unlikely that people using CONFIG_RANDOMIZE_BASE won't
want the modules base randomized too. I think this is a safe default,
but if you see it as a requirement, I can change it.
> 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 :(
"nokaslr" is already documented. Do you mean adding a note about
modules as well to the existing documentation?
>> +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.
I'd like to make sure this is running well after the pRNG is up and
running. I can run some tests to see how the entropy looks if this is
done during __init, though.
>
>> +#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
>
Thanks for the review!
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists