Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1397367 > unrolled thread

[PATCH v7 0/9] x86/KASLR: Randomize virtual address separately

Started byKees Cook <keescook@chromium.org>
First post2016-05-09 22:30 +0200
Last post2016-05-10 10:50 +0200
Articles 13 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v7 0/9] x86/KASLR: Randomize virtual address separately Kees Cook <keescook@chromium.org> - 2016-05-09 22:30 +0200
    [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time Kees Cook <keescook@chromium.org> - 2016-05-09 22:30 +0200
      Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time Yinghai Lu <yinghai@kernel.org> - 2016-05-10 00:10 +0200
        Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time Yinghai Lu <yinghai@kernel.org> - 2016-05-10 00:30 +0200
        Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time Kees Cook <keescook@chromium.org> - 2016-05-10 00:30 +0200
          Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time Yinghai Lu <yinghai@kernel.org> - 2016-05-10 00:30 +0200
            Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time Ingo Molnar <mingo@kernel.org> - 2016-05-10 08:00 +0200
              Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time Borislav Petkov <bp@suse.de> - 2016-05-10 13:30 +0200
        Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time Yinghai Lu <yinghai@kernel.org> - 2016-05-10 00:30 +0200
      [tip:x86/boot] x86/KASLR: Initialize mapping_info every time tip-bot for Kees Cook <tipbot@zytor.com> - 2016-05-10 10:50 +0200
    [PATCH v7 8/9] x86/KASLR: Add physical address randomization >4G Kees Cook <keescook@chromium.org> - 2016-05-09 22:30 +0200
    [PATCH v7 2/9] x86/boot: Add missing file header comments Kees Cook <keescook@chromium.org> - 2016-05-09 22:30 +0200
      [tip:x86/boot] x86/boot: Add missing file header comments tip-bot for Kees Cook <tipbot@zytor.com> - 2016-05-10 10:50 +0200

#1397367 — [PATCH v7 0/9] x86/KASLR: Randomize virtual address separately

FromKees Cook <keescook@chromium.org>
Date2016-05-09 22:30 +0200
Subject[PATCH v7 0/9] x86/KASLR: Randomize virtual address separately
Message-ID<rwZYt-4gl-5@gated-at.bofh.it>
Okay, here's the remaining patches for this series. Hopefully these
are clean and obvious. Technically this is v7 of the series, though
much of it has already been added to -tip already. This is all based on
the work of Yinghai, Baoquan, and myself.

***Background:
Bugs have been reported around kdump, kexec, and some netboot situations
that didn't work when KASLR was enabled. While discussing the bugs, it
was found that the current KASLR implementation had various limitations,
but most importantly that it can only randomize in a 1GB region of
physical memory.

The current KASLR implementaion only randomizes the base physical
address of the kernel. If the delta from build-time load address and
KASLR run-time load address (i.e. the physical address of where the
kernel actually decompressed) is not equal to 0, relocation handling is
performed using the delta. Though in principle kernel can be randomized
to any physical address, the physical kernel text mapping address space
is limited to 1G and the virtual address is just offset by the same
amount. On x86_64 the result is the following range:
[0xffffffff80000000, 0xffffffffc0000000)

hpa and Vivek suggested we should change this by decoupling the physical
address and virtual address randomization of kernel text and let them work
separately. Then kernel text physical address can be randomized in region
[16M, 64T), and kernel text virtual address can be randomized in region
[0xffffffff80000000, 0xffffffffc0000000).

***Problems that needed solving:
  - When booting from the startup_32 case, only a 0~4G identity mapping is
    built. If kernel will be randomly put anywhere from 16M to 64T at
    most, the price to build all the identity mappings is too high. We
    need to build the identity mapping on demand, not covering all of the
    physical address space. (The infrastructure for solving this has
    already landed in -tip now.)

  - Decouple the physical address and virtual address randomization of kernel
    text and let them work separately.

  - Kernels loaded high will not randomize into a lower memory region.

***Parts:
   - The 1st part includes a bug fix and clean ups.
     (Patches 1-2)
   - The 2nd part is Baoquan's new randomization slot management code for
     handling the much larger possible memory space.
     (Patches 3-4)
   - The 4th part is Baoquan's decoupling the physical address and virtual
     address randomization of kernel text and letting them work separately,
     based on Yinghai's ident mapping patches, fixing problem #2 above.
     (Patches 5-7)
   - The 5th part lifts the upper and lower limits on physical addresses,
     fixing problem #3 above.
     (Patches 8-9)

I've boot tested this a bunch on 32-bit and 64-bit, and things appear to
be working as expected. I've cleaned up the changelogs, improved some
comments, refactored a few things, split out things and merged others,
etc. Changes are noted in the individual changelogs.

Thanks!

-Kees

v6->v7:
- general refactoring to adapt to changing casts, etc in earlier patches.
- found and fixed a typo in my refactorings that was obscuring a bug in
  my changes to the page table ident mapping code (physical address was
  never being added to identity maps).
- found some more file header comments to add, fixed mem_avoid comment typo.

v5->v6:
- sent other clean-ups as separate patches
- squashed code removal into the patches that made them removable
- refactoring slot calculation to avoid the confusing "if" statement
- protected slot_area_index in store_slot_info to be paranoid
- adjusted Kconfig language to be more complete but with hopefully less jargon
- fixed up as much of the unsigned long casts as possible
- fixed some coding styel on brace usage
- made find_random_phys_addr look like find_random_virt_addr
- clarified various variable names

v4->v5:
- rewrote all the changelogs, and several comments.
- refactored e820 parser to use a while loop instead of goto.
- rearranged removal of CONFIG_RANDOMIZE_BASE_MAX_OFFSET to earlier.
- additionally dropped KERNEL_IMAGE_SIZE_DEFAULT
- refactored minimum address calculation
- refactored slot offset calculation for readability
- fixed 32-bit boot failures
- fixed CONFIG_RANDOMIZE_BASE=n boot failure
- improved debug reporting

[Baoquan's histroy]
v3->v4:
- Made changes according to Kees's comments.
  Add one patch 20/20 as Kees suggested to use KERNEL_IMAGE_SIZE as offset
  max of virtual random, meanwhile clean up useless CONFIG_RANDOM_OFFSET_MAX

    x86, kaslr: Use KERNEL_IMAGE_SIZE as the offset max for kernel virtual randomization

v2->v3:
- It only takes care of the kaslr related patches.
  For reviewers it's better to discuss only one issue in one thread.
    * I take off one patch as follows from Yinghai's because I think it's unnecessay.
       - Patch 05/19 x86, kaslr: rename output_size to output_run_size
         output_size is enough to represen the value:
            output_len > run_size ? output_len : run_size

    * I add Patch 04/19, it's a comment update patch. For other patches, I just
      adjust patch log and do several places of change comparing with 2nd round.
      Please check the change log under patch log of each patch for details.

    * Adjust sequence of several patches to make review easier. It doesn't
      affect codes.

v1->v2:
- In 2nd round Yinghai made a big patchset including this kaslr fix and another
  setup_data related fix. The link is here:
   http://lists-archives.com/linux-kernel/28346903-x86-updated-patches-for-kaslr-and-setup_data-etc-for-v4-3.html
  You can get the code from Yinghai's git branch:
  git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-x86-v4.3-next

v1:
- The first round can be found here:
    https://lwn.net/Articles/637115/

[toc] | [next] | [standalone]


#1397368 — [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time

FromKees Cook <keescook@chromium.org>
Date2016-05-09 22:30 +0200
Subject[PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time
Message-ID<rwZYv-4gl-47@gated-at.bofh.it>
In reply to#1397367
As it turns out, mapping_info DOES need to be initialized every
time. Without this, page tables were not being corrected updated, which
could cause reboots when a physical address beyond 2G was chosen.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/boot/compressed/pagetable.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/compressed/pagetable.c b/arch/x86/boot/compressed/pagetable.c
index 3c99051566a9..34b95df14e69 100644
--- a/arch/x86/boot/compressed/pagetable.c
+++ b/arch/x86/boot/compressed/pagetable.c
@@ -90,23 +90,17 @@ static void prepare_level4(void)
 }
 
 /*
- * Mapping information structure passed to kernel_ident_mapping_init().
- * Since this never changes, there's no reason to repeatedly fill it
- * in on the stack when calling add_identity_map().
- */
-static struct x86_mapping_info mapping_info = {
-	.alloc_pgt_page	= alloc_pgt_page,
-	.context	= &pgt_data,
-	.pmd_flag	= __PAGE_KERNEL_LARGE_EXEC,
-};
-
-/*
  * Adds the specified range to what will become the new identity mappings.
  * Once all ranges have been added, the new mapping is activated by calling
  * finalize_identity_maps() below.
  */
 void add_identity_map(unsigned long start, unsigned long size)
 {
+	struct x86_mapping_info mapping_info = {
+		.alloc_pgt_page	= alloc_pgt_page,
+		.context	= &pgt_data,
+		.pmd_flag	= __PAGE_KERNEL_LARGE_EXEC,
+	};
 	unsigned long end = start + size;
 
 	/* Make sure we have a top level page table ready to use. */
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]


#1397454 — Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time

FromYinghai Lu <yinghai@kernel.org>
Date2016-05-10 00:10 +0200
SubjectRe: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time
Message-ID<rx1xh-6ah-39@gated-at.bofh.it>
In reply to#1397368
On Mon, May 9, 2016 at 1:22 PM, Kees Cook <keescook@chromium.org> wrote:
> As it turns out, mapping_info DOES need to be initialized every
> time. Without this, page tables were not being corrected updated, which
> could cause reboots when a physical address beyond 2G was chosen.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  arch/x86/boot/compressed/pagetable.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/pagetable.c b/arch/x86/boot/compressed/pagetable.c
> index 3c99051566a9..34b95df14e69 100644
> --- a/arch/x86/boot/compressed/pagetable.c
> +++ b/arch/x86/boot/compressed/pagetable.c
> @@ -90,23 +90,17 @@ static void prepare_level4(void)
>  }
>
>  /*
> - * Mapping information structure passed to kernel_ident_mapping_init().
> - * Since this never changes, there's no reason to repeatedly fill it
> - * in on the stack when calling add_identity_map().
> - */
> -static struct x86_mapping_info mapping_info = {
> -       .alloc_pgt_page = alloc_pgt_page,
> -       .context        = &pgt_data,
> -       .pmd_flag       = __PAGE_KERNEL_LARGE_EXEC,
> -};
> -
> -/*
>   * Adds the specified range to what will become the new identity mappings.
>   * Once all ranges have been added, the new mapping is activated by calling
>   * finalize_identity_maps() below.
>   */
>  void add_identity_map(unsigned long start, unsigned long size)
>  {
> +       struct x86_mapping_info mapping_info = {
> +               .alloc_pgt_page = alloc_pgt_page,
> +               .context        = &pgt_data,
> +               .pmd_flag       = __PAGE_KERNEL_LARGE_EXEC,
> +       };
>         unsigned long end = start + size;

Still should be assigned once, and should be done in
prepare_level4(). --- by may need different name for that ?

The exact reason to have assigning in functions.

pgt_data address could be changed during kernel relocation.
so can not assigned during compiling time.

Thanks

Yinghai

[toc] | [prev] | [next] | [standalone]


#1397470 — Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time

FromYinghai Lu <yinghai@kernel.org>
Date2016-05-10 00:30 +0200
SubjectRe: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time
Message-ID<rx1QC-6kP-7@gated-at.bofh.it>
In reply to#1397454
On Mon, May 9, 2016 at 3:23 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, May 9, 2016 at 3:01 PM, Yinghai Lu <yinghai@kernel.org> wrote:

>> Still should be assigned once, and should be done in
>> prepare_level4(). --- by may need different name for that ?
>>
>> The exact reason to have assigning in functions.
>>
>> pgt_data address could be changed during kernel relocation.
>> so can not assigned during compiling time.
>
> Something like:
>
> -/*
> - * Mapping information structure passed to kernel_ident_mapping_init().
> - * Since this never changes, there's no reason to repeatedly fill it
> - * in on the stack when calling add_identity_map().
> - */
> -static struct x86_mapping_info mapping_info = {
> -       .alloc_pgt_page = alloc_pgt_page,
> -       .context        = &pgt_data,
> -       .pmd_flag       = __PAGE_KERNEL_LARGE_EXEC,
> -};
> +static struct x86_mapping_info mapping_info;
>
>  /*
>   * Adds the specified range to what will become the new identity mappings.
> @@ -110,8 +101,14 @@ void add_identity_map(unsigned long start,
> unsigned long size)
>         unsigned long end = start + size;
>
>         /* Make sure we have a top level page table ready to use. */
> -       if (!level4p)
> +       if (!level4p) {
> +               /* need to set once during run time */
> +               mapping_info.alloc_pgt_page     = alloc_pgt_page;
> +               mapping_info.context    = &pgt_data;
> +               mapping_info.pmd_flag   = __PAGE_KERNEL_LARGE_EXEC;
> +
>                 prepare_level4();
> +       }
>
>         /* Align boundary to 2M. */
>         start = round_down(start, PMD_SIZE);

It would be better if Ingo could fold the change to

https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/patch/?id=3a94707d7a7bb1eb82acae5fbc035247dd1ba8a5
x86/KASLR: Build identity mappings on demand

As it happens to the last commit in tip/x86/boot.

Thanks

Yinghai

[toc] | [prev] | [next] | [standalone]


#1397475 — Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time

FromKees Cook <keescook@chromium.org>
Date2016-05-10 00:30 +0200
SubjectRe: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time
Message-ID<rx1QD-6kP-33@gated-at.bofh.it>
In reply to#1397454
On Mon, May 9, 2016 at 3:01 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, May 9, 2016 at 1:22 PM, Kees Cook <keescook@chromium.org> wrote:
>> As it turns out, mapping_info DOES need to be initialized every
>> time. Without this, page tables were not being corrected updated, which
>> could cause reboots when a physical address beyond 2G was chosen.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  arch/x86/boot/compressed/pagetable.c | 16 +++++-----------
>>  1 file changed, 5 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/pagetable.c b/arch/x86/boot/compressed/pagetable.c
>> index 3c99051566a9..34b95df14e69 100644
>> --- a/arch/x86/boot/compressed/pagetable.c
>> +++ b/arch/x86/boot/compressed/pagetable.c
>> @@ -90,23 +90,17 @@ static void prepare_level4(void)
>>  }
>>
>>  /*
>> - * Mapping information structure passed to kernel_ident_mapping_init().
>> - * Since this never changes, there's no reason to repeatedly fill it
>> - * in on the stack when calling add_identity_map().
>> - */
>> -static struct x86_mapping_info mapping_info = {
>> -       .alloc_pgt_page = alloc_pgt_page,
>> -       .context        = &pgt_data,
>> -       .pmd_flag       = __PAGE_KERNEL_LARGE_EXEC,
>> -};
>> -
>> -/*
>>   * Adds the specified range to what will become the new identity mappings.
>>   * Once all ranges have been added, the new mapping is activated by calling
>>   * finalize_identity_maps() below.
>>   */
>>  void add_identity_map(unsigned long start, unsigned long size)
>>  {
>> +       struct x86_mapping_info mapping_info = {
>> +               .alloc_pgt_page = alloc_pgt_page,
>> +               .context        = &pgt_data,
>> +               .pmd_flag       = __PAGE_KERNEL_LARGE_EXEC,
>> +       };
>>         unsigned long end = start + size;
>
> Still should be assigned once, and should be done in
> prepare_level4(). --- by may need different name for that ?
>
> The exact reason to have assigning in functions.
>
> pgt_data address could be changed during kernel relocation.
> so can not assigned during compiling time.

Ah-ha, that explains why I had to keep it on the stack. Thanks for the
clarification!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]


#1397476 — Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time

FromYinghai Lu <yinghai@kernel.org>
Date2016-05-10 00:30 +0200
SubjectRe: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time
Message-ID<rx1QD-6kP-35@gated-at.bofh.it>
In reply to#1397475
On Mon, May 9, 2016 at 3:23 PM, Kees Cook <keescook@chromium.org> wrote:
>> The exact reason to have assigning in functions.
>>
>> pgt_data address could be changed during kernel relocation.
>> so can not assigned during compiling time.
>
> Ah-ha, that explains why I had to keep it on the stack. Thanks for the
> clarification!

My fault, I should put that comment in the code at first place.

Thanks

Yinghai

[toc] | [prev] | [next] | [standalone]


#1397720 — Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time

FromIngo Molnar <mingo@kernel.org>
Date2016-05-10 08:00 +0200
SubjectRe: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time
Message-ID<rx8S5-4qk-9@gated-at.bofh.it>
In reply to#1397476
* Yinghai Lu <yinghai@kernel.org> wrote:

> On Mon, May 9, 2016 at 3:23 PM, Kees Cook <keescook@chromium.org> wrote:
> >> The exact reason to have assigning in functions.
> >>
> >> pgt_data address could be changed during kernel relocation.
> >> so can not assigned during compiling time.
> >
> > Ah-ha, that explains why I had to keep it on the stack. Thanks for the
> > clarification!
> 
> My fault, I should put that comment in the code at first place.

Ok, could you guys please update the code and the changelog as well, and also 
rename that horribly named prepare_level4() function?

Also, since this code is multi-author, it would be nice to add explicit copyright 
notices to the new arch/x86/boot/compressed/pagetable.c file, it was originally 
written by you, then modified by Kees. Something like:

/*
 * Copyright (C) 2015-2016  Yinghai Lu
 * Copyright (C)      2016  Kees Cook
 */

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]


#1397999 — Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time

FromBorislav Petkov <bp@suse.de>
Date2016-05-10 13:30 +0200
SubjectRe: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time
Message-ID<rxe1s-1qz-9@gated-at.bofh.it>
In reply to#1397720
On Tue, May 10, 2016 at 07:58:54AM +0200, Ingo Molnar wrote:
> Ok, could you guys please update the code and the changelog as well,
> and also rename that horribly named prepare_level4() function?

Oh, and while at it, call that new function at the beginning of
choose_random_location() before the first call to add_identity_map()
through mem_avoid_init() and make the code flow even clearer this way
instead of doing funky checks like

        if (!level4p)
                prepare_level4();

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[toc] | [prev] | [next] | [standalone]


#1397479 — Re: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time

FromYinghai Lu <yinghai@kernel.org>
Date2016-05-10 00:30 +0200
SubjectRe: [PATCH v7 1/9] x86/KASLR: Initialize mapping_info every time
Message-ID<rx1QC-6kP-9@gated-at.bofh.it>
In reply to#1397454
On Mon, May 9, 2016 at 3:01 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, May 9, 2016 at 1:22 PM, Kees Cook <keescook@chromium.org> wrote:
>> As it turns out, mapping_info DOES need to be initialized every
>> time. Without this, page tables were not being corrected updated, which
>> could cause reboots when a physical address beyond 2G was chosen.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  arch/x86/boot/compressed/pagetable.c | 16 +++++-----------
>>  1 file changed, 5 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/pagetable.c b/arch/x86/boot/compressed/pagetable.c
>> index 3c99051566a9..34b95df14e69 100644
>> --- a/arch/x86/boot/compressed/pagetable.c
>> +++ b/arch/x86/boot/compressed/pagetable.c
>> @@ -90,23 +90,17 @@ static void prepare_level4(void)
>>  }
>>
>>  /*
>> - * Mapping information structure passed to kernel_ident_mapping_init().
>> - * Since this never changes, there's no reason to repeatedly fill it
>> - * in on the stack when calling add_identity_map().
>> - */
>> -static struct x86_mapping_info mapping_info = {
>> -       .alloc_pgt_page = alloc_pgt_page,
>> -       .context        = &pgt_data,
>> -       .pmd_flag       = __PAGE_KERNEL_LARGE_EXEC,
>> -};
>> -
>> -/*
>>   * Adds the specified range to what will become the new identity mappings.
>>   * Once all ranges have been added, the new mapping is activated by calling
>>   * finalize_identity_maps() below.
>>   */
>>  void add_identity_map(unsigned long start, unsigned long size)
>>  {
>> +       struct x86_mapping_info mapping_info = {
>> +               .alloc_pgt_page = alloc_pgt_page,
>> +               .context        = &pgt_data,
>> +               .pmd_flag       = __PAGE_KERNEL_LARGE_EXEC,
>> +       };
>>         unsigned long end = start + size;
>
> Still should be assigned once, and should be done in
> prepare_level4(). --- by may need different name for that ?
>
> The exact reason to have assigning in functions.
>
> pgt_data address could be changed during kernel relocation.
> so can not assigned during compiling time.

Something like:

-/*
- * Mapping information structure passed to kernel_ident_mapping_init().
- * Since this never changes, there's no reason to repeatedly fill it
- * in on the stack when calling add_identity_map().
- */
-static struct x86_mapping_info mapping_info = {
-       .alloc_pgt_page = alloc_pgt_page,
-       .context        = &pgt_data,
-       .pmd_flag       = __PAGE_KERNEL_LARGE_EXEC,
-};
+static struct x86_mapping_info mapping_info;

 /*
  * Adds the specified range to what will become the new identity mappings.
@@ -110,8 +101,14 @@ void add_identity_map(unsigned long start,
unsigned long size)
        unsigned long end = start + size;

        /* Make sure we have a top level page table ready to use. */
-       if (!level4p)
+       if (!level4p) {
+               /* need to set once during run time */
+               mapping_info.alloc_pgt_page     = alloc_pgt_page;
+               mapping_info.context    = &pgt_data;
+               mapping_info.pmd_flag   = __PAGE_KERNEL_LARGE_EXEC;
+
                prepare_level4();
+       }

        /* Align boundary to 2M. */
        start = round_down(start, PMD_SIZE);

[toc] | [prev] | [next] | [standalone]


#1397860 — [tip:x86/boot] x86/KASLR: Initialize mapping_info every time

Fromtip-bot for Kees Cook <tipbot@zytor.com>
Date2016-05-10 10:50 +0200
Subject[tip:x86/boot] x86/KASLR: Initialize mapping_info every time
Message-ID<rxbwC-7k3-21@gated-at.bofh.it>
In reply to#1397368
Commit-ID:  434a6c9f90f7ab5ade619455df01ef5ebea533ee
Gitweb:     http://git.kernel.org/tip/434a6c9f90f7ab5ade619455df01ef5ebea533ee
Author:     Kees Cook <keescook@chromium.org>
AuthorDate: Mon, 9 May 2016 13:22:04 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 10 May 2016 10:12:02 +0200

x86/KASLR: Initialize mapping_info every time

As it turns out, mapping_info DOES need to be initialized every
time, because pgt_data address could be changed during kernel
relocation. So it can not be build time assigned.

Without this, page tables were not being corrected updated, which
could cause reboots when a physical address beyond 2G was chosen.

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: kernel-hardening@lists.openwall.com
Cc: lasse.collin@tukaani.org
Link: http://lkml.kernel.org/r/1462825332-10505-2-git-send-email-keescook@chromium.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/boot/compressed/pagetable.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/compressed/pagetable.c b/arch/x86/boot/compressed/pagetable.c
index 3c99051..34b95df 100644
--- a/arch/x86/boot/compressed/pagetable.c
+++ b/arch/x86/boot/compressed/pagetable.c
@@ -90,23 +90,17 @@ static void prepare_level4(void)
 }
 
 /*
- * Mapping information structure passed to kernel_ident_mapping_init().
- * Since this never changes, there's no reason to repeatedly fill it
- * in on the stack when calling add_identity_map().
- */
-static struct x86_mapping_info mapping_info = {
-	.alloc_pgt_page	= alloc_pgt_page,
-	.context	= &pgt_data,
-	.pmd_flag	= __PAGE_KERNEL_LARGE_EXEC,
-};
-
-/*
  * Adds the specified range to what will become the new identity mappings.
  * Once all ranges have been added, the new mapping is activated by calling
  * finalize_identity_maps() below.
  */
 void add_identity_map(unsigned long start, unsigned long size)
 {
+	struct x86_mapping_info mapping_info = {
+		.alloc_pgt_page	= alloc_pgt_page,
+		.context	= &pgt_data,
+		.pmd_flag	= __PAGE_KERNEL_LARGE_EXEC,
+	};
 	unsigned long end = start + size;
 
 	/* Make sure we have a top level page table ready to use. */

[toc] | [prev] | [next] | [standalone]


#1397370 — [PATCH v7 8/9] x86/KASLR: Add physical address randomization >4G

FromKees Cook <keescook@chromium.org>
Date2016-05-09 22:30 +0200
Subject[PATCH v7 8/9] x86/KASLR: Add physical address randomization >4G
Message-ID<rwZYw-4gl-53@gated-at.bofh.it>
In reply to#1397367
This patch exchanges the prior slots[] array for the new slot_areas[]
array, and lifts the limitation of KERNEL_IMAGE_SIZE on the physical
address offset for 64-bit. As before, process_e820_entry() walks
memory and populates slot_areas[], splitting on any detected mem_avoid
collisions.

Finally, since the slots[] array and its associated functions are not
needed any more, so they are removed.

Based on earlier patches by Baoquan He.

Cc: Baoquan He <bhe@redhat.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
This patch is pretty noisy due to the indentation change in the
e820 walker. I couldn't find a cleaner way to do this that didn't
make the final code LESS readable, unfortunately. So, the diff is
ugly, but I think the results are clean.
---
 arch/x86/Kconfig                 |  27 +++++----
 arch/x86/boot/compressed/kaslr.c | 115 +++++++++++++++++++++++----------------
 2 files changed, 85 insertions(+), 57 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5892d549596d..bba81cf02b69 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1943,21 +1943,26 @@ config RANDOMIZE_BASE
 	  attempts relying on knowledge of the location of kernel
 	  code internals.
 
-	  The kernel physical and virtual address can be randomized
-	  from 16MB up to 1GB on 64-bit and 512MB on 32-bit. (Note that
-	  using RANDOMIZE_BASE reduces the memory space available to
-	  kernel modules from 1.5GB to 1GB.)
+	  On 64-bit, the kernel physical and virtual addresses are
+	  randomized separately. The physical address will be anywhere
+	  between 16MB and the top of physical memory (up to 64TB). The
+	  virtual address will be randomized from 16MB up to 1GB (9 bits
+	  of entropy). Note that this also reduces the memory space
+	  available to kernel modules from 1.5GB to 1GB.
+
+	  On 32-bit, the kernel physical and virtual addresses are
+	  randomized together. They will be randomized from 16MB up to
+	  512MB (8 bits of entropy).
 
 	  Entropy is generated using the RDRAND instruction if it is
 	  supported. If RDTSC is supported, its value is mixed into
 	  the entropy pool as well. If neither RDRAND nor RDTSC are
-	  supported, then entropy is read from the i8254 timer.
-
-	  Since the kernel is built using 2GB addressing, and
-	  PHYSICAL_ALIGN must be at a minimum of 2MB, only 10 bits of
-	  entropy is theoretically possible. Currently, with the
-	  default value for PHYSICAL_ALIGN and due to page table
-	  layouts, 64-bit uses 9 bits of entropy and 32-bit uses 8 bits.
+	  supported, then entropy is read from the i8254 timer. The
+	  usable entropy is limited by the kernel being built using
+	  2GB addressing, and that PHYSICAL_ALIGN must be at a
+	  minimum of 2MB. As a result, only 10 bits of entropy are
+	  theoretically possible, but the implementations are further
+	  limited due to memory layouts.
 
 	  If CONFIG_HIBERNATE is also enabled, KASLR is disabled at boot
 	  time. To enable it, boot with "kaslr" on the kernel command
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 1d1338f04bbd..def1da9e7863 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -132,17 +132,6 @@ enum mem_avoid_index {
 
 static struct mem_vector mem_avoid[MEM_AVOID_MAX];
 
-static bool mem_contains(struct mem_vector *region, struct mem_vector *item)
-{
-	/* Item at least partially before region. */
-	if (item->start < region->start)
-		return false;
-	/* Item at least partially after region. */
-	if (item->start + item->size > region->start + region->size)
-		return false;
-	return true;
-}
-
 static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
 {
 	/* Item one is entirely before item two. */
@@ -319,8 +308,6 @@ static bool mem_avoid_overlap(struct mem_vector *img,
 	return is_overlapping;
 }
 
-static unsigned long slots[KERNEL_IMAGE_SIZE / CONFIG_PHYSICAL_ALIGN];
-
 struct slot_area {
 	unsigned long addr;
 	int num;
@@ -351,36 +338,44 @@ static void store_slot_info(struct mem_vector *region, unsigned long image_size)
 	}
 }
 
-static void slots_append(unsigned long addr)
-{
-	/* Overflowing the slots list should be impossible. */
-	if (slot_max >= KERNEL_IMAGE_SIZE / CONFIG_PHYSICAL_ALIGN)
-		return;
-
-	slots[slot_max++] = addr;
-}
-
 static unsigned long slots_fetch_random(void)
 {
+	unsigned long slot;
+	int i;
+
 	/* Handle case of no slots stored. */
 	if (slot_max == 0)
 		return 0;
 
-	return slots[get_random_long("Physical") % slot_max];
+	slot = get_random_long("Physical") % slot_max;
+
+	for (i = 0; i < slot_area_index; i++) {
+		if (slot >= slot_areas[i].num) {
+			slot -= slot_areas[i].num;
+			continue;
+		}
+		return slot_areas[i].addr + slot * CONFIG_PHYSICAL_ALIGN;
+	}
+
+	if (i == slot_area_index)
+		debug_putstr("slots_fetch_random() failed!?\n");
+	return 0;
 }
 
 static void process_e820_entry(struct e820entry *entry,
 			       unsigned long minimum,
 			       unsigned long image_size)
 {
-	struct mem_vector region, img, overlap;
+	struct mem_vector region, overlap;
+	struct slot_area slot_area;
+	unsigned long start_orig;
 
 	/* Skip non-RAM entries. */
 	if (entry->type != E820_RAM)
 		return;
 
-	/* Ignore entries entirely above our maximum. */
-	if (entry->addr >= KERNEL_IMAGE_SIZE)
+	/* On 32-bit, ignore entries entirely above our maximum. */
+	if (IS_ENABLED(CONFIG_X86_32) && entry->addr >= KERNEL_IMAGE_SIZE)
 		return;
 
 	/* Ignore entries entirely below our minimum. */
@@ -390,31 +385,55 @@ static void process_e820_entry(struct e820entry *entry,
 	region.start = entry->addr;
 	region.size = entry->size;
 
-	/* Potentially raise address to minimum location. */
-	if (region.start < minimum)
-		region.start = minimum;
+	/* Give up if slot area array is full. */
+	while (slot_area_index < MAX_SLOT_AREA) {
+		start_orig = region.start;
 
-	/* Potentially raise address to meet alignment requirements. */
-	region.start = ALIGN(region.start, CONFIG_PHYSICAL_ALIGN);
+		/* Potentially raise address to minimum location. */
+		if (region.start < minimum)
+			region.start = minimum;
 
-	/* Did we raise the address above the bounds of this e820 region? */
-	if (region.start > entry->addr + entry->size)
-		return;
+		/* Potentially raise address to meet alignment needs. */
+		region.start = ALIGN(region.start, CONFIG_PHYSICAL_ALIGN);
 
-	/* Reduce size by any delta from the original address. */
-	region.size -= region.start - entry->addr;
+		/* Did we raise the address above this e820 region? */
+		if (region.start > entry->addr + entry->size)
+			return;
 
-	/* Reduce maximum size to fit end of image within maximum limit. */
-	if (region.start + region.size > KERNEL_IMAGE_SIZE)
-		region.size = KERNEL_IMAGE_SIZE - region.start;
+		/* Reduce size by any delta from the original address. */
+		region.size -= region.start - start_orig;
 
-	/* Walk each aligned slot and check for avoided areas. */
-	for (img.start = region.start, img.size = image_size ;
-	     mem_contains(&region, &img) ;
-	     img.start += CONFIG_PHYSICAL_ALIGN) {
-		if (mem_avoid_overlap(&img, &overlap))
-			continue;
-		slots_append(img.start);
+		/* On 32-bit, reduce region size to fit within max size. */
+		if (IS_ENABLED(CONFIG_X86_32) &&
+		    region.start + region.size > KERNEL_IMAGE_SIZE)
+			region.size = KERNEL_IMAGE_SIZE - region.start;
+
+		/* Return if region can't contain decompressed kernel */
+		if (region.size < image_size)
+			return;
+
+		/* If nothing overlaps, store the region and return. */
+		if (!mem_avoid_overlap(&region, &overlap)) {
+			store_slot_info(&region, image_size);
+			return;
+		}
+
+		/* Store beginning of region if holds at least image_size. */
+		if (overlap.start > region.start + image_size) {
+			struct mem_vector beginning;
+
+			beginning.start = region.start;
+			beginning.size = overlap.start - region.start;
+			store_slot_info(&beginning, image_size);
+		}
+
+		/* Return if overlap extends to or past end of region. */
+		if (overlap.start + overlap.size >= region.start + region.size)
+			return;
+
+		/* Clip off the overlapping region and start over. */
+		region.size -= overlap.start - region.start + overlap.size;
+		region.start = overlap.start + overlap.size;
 	}
 }
 
@@ -431,6 +450,10 @@ static unsigned long find_random_phys_addr(unsigned long minimum,
 	for (i = 0; i < boot_params->e820_entries; i++) {
 		process_e820_entry(&boot_params->e820_map[i], minimum,
 				   image_size);
+		if (slot_area_index == MAX_SLOT_AREA) {
+			debug_putstr("Aborted e820 scan (slot_areas full)!\n");
+			break;
+		}
 	}
 
 	return slots_fetch_random();
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]


#1397371 — [PATCH v7 2/9] x86/boot: Add missing file header comments

FromKees Cook <keescook@chromium.org>
Date2016-05-09 22:30 +0200
Subject[PATCH v7 2/9] x86/boot: Add missing file header comments
Message-ID<rwZYw-4gl-57@gated-at.bofh.it>
In reply to#1397367
There were some files with missing header comments. Since they are
included from both compressed and regular kernels, make note of that.
Also corrects a typo in the mem_avoid comments.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/boot/compressed/kaslr.c     | 2 +-
 arch/x86/boot/early_serial_console.c | 4 ++++
 arch/x86/mm/ident_map.c              | 5 +++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index f82975b0f9d6..3f0692dcd30d 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -163,7 +163,7 @@ static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
  * memory ranges lead to really hard to debug boot failures.
  *
  * The initrd, cmdline, and boot_params are trivial to identify for
- * avoiding. The are MEM_AVOID_INITRD, MEM_AVOID_CMDLINE, and
+ * avoiding. They are MEM_AVOID_INITRD, MEM_AVOID_CMDLINE, and
  * MEM_AVOID_BOOTPARAMS respectively below.
  *
  * What is not obvious how to avoid is the range of memory that is used
diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index 45a07684bbab..f0b8d6d93164 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -1,3 +1,7 @@
+/*
+ * Serial port routines for use during early boot reporting. This code is
+ * included from both the compressed kernel and the regular kernel.
+ */
 #include "boot.h"
 
 #define DEFAULT_SERIAL_PORT 0x3f8 /* ttyS0 */
diff --git a/arch/x86/mm/ident_map.c b/arch/x86/mm/ident_map.c
index 751ca920773a..ec21796ac5fd 100644
--- a/arch/x86/mm/ident_map.c
+++ b/arch/x86/mm/ident_map.c
@@ -1,3 +1,7 @@
+/*
+ * Helper routines for building identity mapping page tables. This is
+ * included by both the compressed kernel and the regular kernel.
+ */
 
 static void ident_pmd_init(unsigned long pmd_flag, pmd_t *pmd_page,
 			   unsigned long addr, unsigned long end)
@@ -10,6 +14,7 @@ static void ident_pmd_init(unsigned long pmd_flag, pmd_t *pmd_page,
 			set_pmd(pmd, __pmd(addr | pmd_flag));
 	}
 }
+
 static int ident_pud_init(struct x86_mapping_info *info, pud_t *pud_page,
 			  unsigned long addr, unsigned long end)
 {
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]


#1397866 — [tip:x86/boot] x86/boot: Add missing file header comments

Fromtip-bot for Kees Cook <tipbot@zytor.com>
Date2016-05-10 10:50 +0200
Subject[tip:x86/boot] x86/boot: Add missing file header comments
Message-ID<rxbwD-7k3-33@gated-at.bofh.it>
In reply to#1397371
Commit-ID:  cb18ef0da259db611fbf52806592fde5f469ae67
Gitweb:     http://git.kernel.org/tip/cb18ef0da259db611fbf52806592fde5f469ae67
Author:     Kees Cook <keescook@chromium.org>
AuthorDate: Mon, 9 May 2016 13:22:05 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 10 May 2016 10:12:03 +0200

x86/boot: Add missing file header comments

There were some files with missing header comments. Since they are
included from both compressed and regular kernels, make note of that.
Also corrects a typo in the mem_avoid comments.

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: kernel-hardening@lists.openwall.com
Cc: lasse.collin@tukaani.org
Link: http://lkml.kernel.org/r/1462825332-10505-3-git-send-email-keescook@chromium.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/boot/compressed/kaslr.c     | 2 +-
 arch/x86/boot/early_serial_console.c | 4 ++++
 arch/x86/mm/ident_map.c              | 5 +++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index f5a138c..f15d7b8 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -163,7 +163,7 @@ static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
  * memory ranges lead to really hard to debug boot failures.
  *
  * The initrd, cmdline, and boot_params are trivial to identify for
- * avoiding. The are MEM_AVOID_INITRD, MEM_AVOID_CMDLINE, and
+ * avoiding. They are MEM_AVOID_INITRD, MEM_AVOID_CMDLINE, and
  * MEM_AVOID_BOOTPARAMS respectively below.
  *
  * What is not obvious how to avoid is the range of memory that is used
diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index 45a0768..f0b8d6d 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -1,3 +1,7 @@
+/*
+ * Serial port routines for use during early boot reporting. This code is
+ * included from both the compressed kernel and the regular kernel.
+ */
 #include "boot.h"
 
 #define DEFAULT_SERIAL_PORT 0x3f8 /* ttyS0 */
diff --git a/arch/x86/mm/ident_map.c b/arch/x86/mm/ident_map.c
index 751ca92..ec21796 100644
--- a/arch/x86/mm/ident_map.c
+++ b/arch/x86/mm/ident_map.c
@@ -1,3 +1,7 @@
+/*
+ * Helper routines for building identity mapping page tables. This is
+ * included by both the compressed kernel and the regular kernel.
+ */
 
 static void ident_pmd_init(unsigned long pmd_flag, pmd_t *pmd_page,
 			   unsigned long addr, unsigned long end)
@@ -10,6 +14,7 @@ static void ident_pmd_init(unsigned long pmd_flag, pmd_t *pmd_page,
 			set_pmd(pmd, __pmd(addr | pmd_flag));
 	}
 }
+
 static int ident_pud_init(struct x86_mapping_info *info, pud_t *pud_page,
 			  unsigned long addr, unsigned long end)
 {

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web