2014-02-21 20:22:58

by Kees Cook

[permalink] [raw]
Subject: [PATCH] x86, kaslr: randomize module base load address

From: Andy Honig <[email protected]>

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 ]---

Signed-off-by: Andy Honig <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
arch/x86/kernel/module.c | 43 ++++++++++++++++++++++++++++++++++++++++---
1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index 18be189368bb..49483137371f 100644
--- 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;
+
+static int __init parse_nokaslr(char *p)
+{
+ randomize_modules = 0;
+ return 0;
+}
+early_param("nokaslr", parse_nokaslr);
+
+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;
+}
+#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
--
1.7.9.5


--
Kees Cook
Chrome OS Security


2014-02-21 20:37:00

by Andrew Morton

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

On Fri, 21 Feb 2014 12:21:10 -0800 Kees Cook <[email protected]> wrote:

> From: Andy Honig <[email protected]>
>
> 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

2014-02-21 21:05:14

by Kees Cook

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

On Fri, Feb 21, 2014 at 12:36 PM, Andrew Morton
<[email protected]> wrote:
> On Fri, 21 Feb 2014 12:21:10 -0800 Kees Cook <[email protected]> wrote:
>
>> From: Andy Honig <[email protected]>
>>
>> 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

2014-02-21 21:10:22

by H. Peter Anvin

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

On 02/21/2014 01:05 PM, Kees Cook wrote:
>
> 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.
>

No, but I could totally see people wanting to randomize modules but not
the main kernel. Why? Because module addresses are already dynamic, so
there is no breakage. Whether or not it is *useful* is another matter.

-hpa

2014-02-21 21:15:35

by Andrew Morton

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

On Fri, 21 Feb 2014 13:05:08 -0800 Kees Cook <[email protected]> wrote:

> >> +#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.

I think there were issues with some embedded systems where it's
hard/impossible to provide/alter boot parameters.

I suppose we can leave it this way until there are complaints.

> > 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?

Yes, it should now mention modules as well.

> >> +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.

That may be a bit optimistic, dunno. I suppose that doing it this way
we will already have done a bit of disk IO, so there will be more
randomness.

btw, would it be better to make each module have its own offset rather
than using the same offset for all of them? That could cause problems
with vmap space fragmentation I guess.

2014-02-21 21:18:49

by H. Peter Anvin

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

On 02/21/2014 01:15 PM, Andrew Morton wrote:
>>
>> 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.
>
> I think there were issues with some embedded systems where it's
> hard/impossible to provide/alter boot parameters.
>

We now allow kernel parameters to be compiled into the kernel image for
that reason.

>
> btw, would it be better to make each module have its own offset rather
> than using the same offset for all of them? That could cause problems
> with vmap space fragmentation I guess.
>

... but it would improve the amount of entropy.

-hpa

2014-02-21 21:21:57

by Kees Cook

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

On Fri, Feb 21, 2014 at 1:15 PM, Andrew Morton
<[email protected]> wrote:
> On Fri, 21 Feb 2014 13:05:08 -0800 Kees Cook <[email protected]> wrote:
>
>> >> +#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.
>
> I think there were issues with some embedded systems where it's
> hard/impossible to provide/alter boot parameters.
>
> I suppose we can leave it this way until there are complaints.
>
>> > 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?
>
> Yes, it should now mention modules as well.

Okay, I will add this to the v2 patch.

>
>> >> +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.
>
> That may be a bit optimistic, dunno. I suppose that doing it this way
> we will already have done a bit of disk IO, so there will be more
> randomness.

Right -- it's probably not much, but this doesn't seem like much
overhead for module loading. It's a relatively infrequent event.

> btw, would it be better to make each module have its own offset rather
> than using the same offset for all of them? That could cause problems
> with vmap space fragmentation I guess.

Right -- we felt this was sufficient. And in my experience, other
attempts and trying to do per-allocat randomization and avoid the
fragmentation problem (e.g. "greedy random positioning"), like done
with RedHat's ASCII-Armor memory randomization proved to have _less_
entropy than just randomizing the base address.

-Kees

--
Kees Cook
Chrome OS Security

2014-02-21 21:27:12

by Andrew Morton

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

On Fri, 21 Feb 2014 13:18:26 -0800 "H. Peter Anvin" <[email protected]> wrote:

> On 02/21/2014 01:15 PM, Andrew Morton wrote:
> >>
> >> 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.
> >
> > I think there were issues with some embedded systems where it's
> > hard/impossible to provide/alter boot parameters.
> >
>
> We now allow kernel parameters to be compiled into the kernel image for
> that reason.

We do? What's that Kconfig variable called?

We have a few (inconsistently named) things like

CONFIG_X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK=y
CONFIG_BOOTPARAM_HOTPLUG_CPU0=y
CONFIG_BOOTPARAM_HARDLOCKUP_PANIC=y
CONFIG_BOOTPARAM_HARDLOCKUP_PANIC_VALUE=1
CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC=y
CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC_VALUE=1
CONFIG_BOOTPARAM_HUNG_TASK_PANIC=y
CONFIG_BOOTPARAM_HUNG_TASK_PANIC_VALUE=1
CONFIG_DEBUG_BOOT_PARAMS=y
CONFIG_SECURITY_SELINUX_BOOTPARAM=y
CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE=1
CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE=1

which presumably become obsolete with a general
feed-this-string-to-the-kernel feature?

2014-02-21 23:02:12

by H. Peter Anvin

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

On 02/21/2014 01:27 PM, Andrew Morton wrote:
> On Fri, 21 Feb 2014 13:18:26 -0800 "H. Peter Anvin" <[email protected]> wrote:
>
>> On 02/21/2014 01:15 PM, Andrew Morton wrote:
>>>>
>>>> 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.
>>>
>>> I think there were issues with some embedded systems where it's
>>> hard/impossible to provide/alter boot parameters.
>>>
>>
>> We now allow kernel parameters to be compiled into the kernel image for
>> that reason.
>
> We do? What's that Kconfig variable called?
>

CONFIG_CMDLINE and CONFIG_CMDLINE_OVERRIDE. They seem to be
x86-specific at this time, although I think some other arches have
similar hacks.

-hpa