Re: [PATCH v3 16/19] x86, kaslr: Randomize physical and virtual address of kernel separately

From: Kees Cook
Date: Mon Mar 07 2016 - 18:52:39 EST


On Fri, Mar 4, 2016 at 8:25 AM, Baoquan He <bhe@xxxxxxxxxx> wrote:
> On x86_64, in old kaslr implementaion only physical address of kernel
> loading is randomized. Then calculate the delta of physical address
> where vmlinux was linked to load and where it is finally loaded. If
> delta is not equal to 0, namely there's a new physical address where
> kernel is actually decompressed, relocation handling need be done. Then
> delta is added to offset of kernel symbol relocation, this makes the
> address of kernel text mapping move delta long.
>
> Here the behavior is changed. Randomize both the physical address
> where kernel is decompressed and the virtual address where kernel text
> is mapped. And relocation handling only depends on virtual address
> randomization. Means if and only if virtual address is randomized to
> a different value, we add the delta to the offset of kernel relocs.
>
> Note that up to now both virtual offset and physical addr randomization
> cann't exceed CONFIG_RANDOMIZE_BASE_MAX_OFFSET, namely 1G.

It seems like CONFIG_RANDOMIZE_BASE_MAX_OFFSET should have been
eliminated when the on-demand page table code was added. Once that was
added, there's no physical max any more. And virtual randomization
should have no max at all.

-Kees

>
> Signed-off-by: Baoquan He <bhe@xxxxxxxxxx>
> ---
> arch/x86/boot/compressed/aslr.c | 46 +++++++++++++++++++++--------------------
> arch/x86/boot/compressed/misc.c | 40 +++++++++++++++++++++--------------
> arch/x86/boot/compressed/misc.h | 19 +++++++++--------
> 3 files changed, 59 insertions(+), 46 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
> index 3730a33..75ea931 100644
> --- a/arch/x86/boot/compressed/aslr.c
> +++ b/arch/x86/boot/compressed/aslr.c
> @@ -349,7 +349,7 @@ static void process_e820_entry(struct e820entry *entry,
> }
> }
>
> -static unsigned long find_random_addr(unsigned long minimum,
> +static unsigned long find_random_phy_addr(unsigned long minimum,
> unsigned long size)
> {
> int i;
> @@ -387,23 +387,24 @@ static unsigned long find_random_virt_offset(unsigned long minimum,
> return random * CONFIG_PHYSICAL_ALIGN + minimum;
> }
>
> -unsigned char *choose_kernel_location(unsigned char *input,
> - unsigned long input_size,
> - unsigned char *output,
> - unsigned long output_size)
> +void choose_kernel_location(unsigned char *input,
> + unsigned long input_size,
> + unsigned char **output,
> + unsigned long output_size,
> + unsigned char **virt_offset)
> {
> - unsigned long choice = (unsigned long)output;
> unsigned long random;
> + *virt_offset = (unsigned char *)LOAD_PHYSICAL_ADDR;
>
> #ifdef CONFIG_HIBERNATION
> if (!cmdline_find_option_bool("kaslr")) {
> debug_putstr("KASLR disabled by default...\n");
> - goto out;
> + return;
> }
> #else
> if (cmdline_find_option_bool("nokaslr")) {
> debug_putstr("KASLR disabled by cmdline...\n");
> - goto out;
> + return;
> }
> #endif
>
> @@ -411,23 +412,24 @@ unsigned char *choose_kernel_location(unsigned char *input,
>
> /* Record the various known unsafe memory ranges. */
> mem_avoid_init((unsigned long)input, input_size,
> - (unsigned long)output);
> + (unsigned long)*output);
>
> /* Walk e820 and find a random address. */
> - random = find_random_addr(choice, output_size);
> - if (!random) {
> + random = find_random_phy_addr((unsigned long)*output, output_size);
> + if (!random)
> debug_putstr("KASLR could not find suitable E820 region...\n");
> - goto out;
> + else {
> + if ((unsigned long)*output != random) {
> + fill_pagetable(random, output_size);
> + switch_pagetable();
> + *output = (unsigned char *)random;
> + }
> }
>
> - /* Always enforce the minimum. */
> - if (random < choice)
> - goto out;
> -
> - choice = random;
> -
> - fill_pagetable(choice, output_size);
> - switch_pagetable();
> -out:
> - return (unsigned char *)choice;
> + /*
> + * Get a random address between LOAD_PHYSICAL_ADDR and
> + * CONFIG_RANDOMIZE_BASE_MAX_OFFSET
> + */
> + random = find_random_virt_offset(LOAD_PHYSICAL_ADDR, output_size);
> + *virt_offset = (unsigned char *)random;
> }
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index 70445c3..362d508 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -251,7 +251,8 @@ void error(char *x)
> }
>
> #if CONFIG_X86_NEED_RELOCS
> -static void handle_relocations(void *output, unsigned long output_len)
> +static void handle_relocations(void *output, unsigned long output_len,
> + void *virt_offset)
> {
> int *reloc;
> unsigned long delta, map, ptr;
> @@ -263,11 +264,6 @@ static void handle_relocations(void *output, unsigned long output_len)
> * and where it was actually loaded.
> */
> delta = min_addr - LOAD_PHYSICAL_ADDR;
> - if (!delta) {
> - debug_putstr("No relocation needed... ");
> - return;
> - }
> - debug_putstr("Performing relocations... ");
>
> /*
> * The kernel contains a table of relocation addresses. Those
> @@ -278,6 +274,22 @@ static void handle_relocations(void *output, unsigned long output_len)
> */
> map = delta - __START_KERNEL_map;
>
> +
> +
> + /*
> + * 32-bit always performs relocations. 64-bit relocations are only
> + * needed if kASLR has chosen a different starting address offset
> + * from __START_KERNEL_map.
> + */
> + if (IS_ENABLED(CONFIG_X86_64))
> + delta = (unsigned long)virt_offset - LOAD_PHYSICAL_ADDR;
> +
> + if (!delta) {
> + debug_putstr("No relocation needed... ");
> + return;
> + }
> + debug_putstr("Performing relocations... ");
> +
> /*
> * Process relocations: 32 bit relocations first then 64 bit after.
> * Three sets of binary relocations are added to the end of the kernel
> @@ -331,7 +343,8 @@ static void handle_relocations(void *output, unsigned long output_len)
> #endif
> }
> #else
> -static inline void handle_relocations(void *output, unsigned long output_len)
> +static inline void handle_relocations(void *output, unsigned long output_len,
> + void *virt_offset)
> { }
> #endif
>
> @@ -397,6 +410,7 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> {
> unsigned long run_size = VO__end - VO__text;
> unsigned char *output_orig = output;
> + unsigned char *virt_offset;
>
> real_mode = rmode;
>
> @@ -434,9 +448,10 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> * the entire decompressed kernel plus relocation table, or the
> * entire decompressed kernel plus .bss and .brk sections.
> */
> - output = choose_kernel_location(input_data, input_len, output,
> + choose_kernel_location(input_data, input_len, &output,
> output_len > run_size ? output_len
> - : run_size);
> + : run_size,
> + &virt_offset);
>
> /* Validate memory location choices. */
> if ((unsigned long)output & (MIN_KERNEL_ALIGN - 1))
> @@ -457,12 +472,7 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
> __decompress(input_data, input_len, NULL, NULL, output, output_len,
> NULL, error);
> parse_elf(output);
> - /*
> - * 32-bit always performs relocations. 64-bit relocations are only
> - * needed if kASLR has chosen a different load address.
> - */
> - if (!IS_ENABLED(CONFIG_X86_64) || output != output_orig)
> - handle_relocations(output, output_len);
> + handle_relocations(output, output_len, virt_offset);
> debug_putstr("done.\nBooting the kernel.\n");
> return output;
> }
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index 39d0e9a..cde6aec 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -69,20 +69,21 @@ int cmdline_find_option_bool(const char *option);
>
> #if CONFIG_RANDOMIZE_BASE
> /* aslr.c */
> -unsigned char *choose_kernel_location(unsigned char *input,
> - unsigned long input_size,
> - unsigned char *output,
> - unsigned long output_size);
> +void choose_kernel_location(unsigned char *input,
> + unsigned long input_size,
> + unsigned char **output,
> + unsigned long output_size,
> + unsigned char **virt_offset);
> /* cpuflags.c */
> bool has_cpuflag(int flag);
> #else
> static inline
> -unsigned char *choose_kernel_location(unsigned char *input,
> - unsigned long input_size,
> - unsigned char *output,
> - unsigned long output_size)
> +void choose_kernel_location(unsigned char *input,
> + unsigned long input_size,
> + unsigned char **output,
> + unsigned long output_size,
> + unsigned char **virt_offset)
> {
> - return output;
> }
> #endif
>
> --
> 2.5.0
>



--
Kees Cook
Chrome OS & Brillo Security