Re: [PATCH 6/5] x86/KASLR: Fix detection 32/64 bit bootloaders for 5-level paging

From: Kees Cook
Date: Wed Jun 28 2017 - 12:40:40 EST


On Wed, Jun 28, 2017 at 5:17 AM, Kirill A. Shutemov
<kirill.shutemov@xxxxxxxxxxxxxxx> wrote:
> KASLR uses hack to detect whether we booted via startup_32() or
> startup_64(): it checks what is loaded into cr3 and compares it to
> _pgtables. _pgtables is the array of page tables where early code
> allocates page table from.
>
> KASLR expects cr3 to point to _pgtables if we booted via startup_32(), but
> that's not true if we booted with 5-level paging enabled. In this case top
> level page table is allocated separately and only the first p4d page table
> is allocated from the array.
>
> Let's modify the check to cover both 4- and 5-level paging cases.
>
> The patch also renames 'level4p' to 'top_level_pgt' as it now can hold
> page table for 4th or 5th level, depending on configuration.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>

This looks good, thanks!

Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>

-Kees

> ---
> arch/x86/boot/compressed/pagetable.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/pagetable.c b/arch/x86/boot/compressed/pagetable.c
> index 8e69df96492e..da4cf44d4aac 100644
> --- a/arch/x86/boot/compressed/pagetable.c
> +++ b/arch/x86/boot/compressed/pagetable.c
> @@ -63,7 +63,7 @@ static void *alloc_pgt_page(void *context)
> static struct alloc_pgt_data pgt_data;
>
> /* The top level page table entry pointer. */
> -static unsigned long level4p;
> +static unsigned long top_level_pgt;
>
> /*
> * Mapping information structure passed to kernel_ident_mapping_init().
> @@ -91,9 +91,15 @@ void initialize_identity_maps(void)
> * If we came here via startup_32(), cr3 will be _pgtable already
> * and we must append to the existing area instead of entirely
> * overwriting it.
> + *
> + * With 5-level paging, we use _pgtable allocate p4d page table,
> + * top-level page table is allocated separately.
> + *
> + * p4d_offset(top_level_pgt, 0) would cover both 4- and 5-level
> + * cases. On 4-level paging it's equal to top_level_pgt.
> */
> - level4p = read_cr3_pa();
> - if (level4p == (unsigned long)_pgtable) {
> + top_level_pgt = read_cr3_pa();
> + if (p4d_offset((pgd_t *)top_level_pgt, 0) == (p4d_t *)_pgtable) {
> debug_putstr("booted via startup_32()\n");
> pgt_data.pgt_buf = _pgtable + BOOT_INIT_PGT_SIZE;
> pgt_data.pgt_buf_size = BOOT_PGT_SIZE - BOOT_INIT_PGT_SIZE;
> @@ -103,7 +109,7 @@ void initialize_identity_maps(void)
> pgt_data.pgt_buf = _pgtable;
> pgt_data.pgt_buf_size = BOOT_PGT_SIZE;
> memset(pgt_data.pgt_buf, 0, pgt_data.pgt_buf_size);
> - level4p = (unsigned long)alloc_pgt_page(&pgt_data);
> + top_level_pgt = (unsigned long)alloc_pgt_page(&pgt_data);
> }
> }
>
> @@ -123,7 +129,7 @@ void add_identity_map(unsigned long start, unsigned long size)
> return;
>
> /* Build the mapping. */
> - kernel_ident_mapping_init(&mapping_info, (pgd_t *)level4p,
> + kernel_ident_mapping_init(&mapping_info, (pgd_t *)top_level_pgt,
> start, end);
> }
>
> @@ -134,5 +140,5 @@ void add_identity_map(unsigned long start, unsigned long size)
> */
> void finalize_identity_maps(void)
> {
> - write_cr3(level4p);
> + write_cr3(top_level_pgt);
> }
> --
> 2.11.0
>



--
Kees Cook
Pixel Security