linux-kernel - Re: [tip:x86/boot] x86/KASLR: Build identity mappings on demand

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160507100541.GA24613@pd.tnic>
Date:	Sat, 7 May 2016 12:05:41 +0200
From:	Borislav Petkov <bp@...en8.de>
To:	Kees Cook <keescook@...omium.org>, Ingo Molnar <mingo@...nel.org>
Cc:	bp@...e.de, jkosina@...e.cz, linux-kernel@...r.kernel.org,
	keescook@...omium.org, brgerst@...il.com, vgoyal@...hat.com,
	akpm@...ux-foundation.org, tglx@...utronix.de, bhe@...hat.com,
	luto@...nel.org, dyoung@...hat.com, yinghai@...nel.org,
	dvlasenk@...hat.com, mingo@...nel.org, peterz@...radead.org,
	luto@...capital.net, torvalds@...ux-foundation.org, hpa@...or.com,
	linux-tip-commits@...r.kernel.org
Subject: Re: [tip:x86/boot] x86/KASLR: Build identity mappings on demand

On Fri, May 06, 2016 at 11:37:22PM -0700, tip-bot for Kees Cook wrote:
> Commit-ID:  3a94707d7a7bb1eb82acae5fbc035247dd1ba8a5
> Gitweb:     http://git.kernel.org/tip/3a94707d7a7bb1eb82acae5fbc035247dd1ba8a5
> Author:     Kees Cook <keescook@...omium.org>
> AuthorDate: Fri, 6 May 2016 15:01:35 -0700
> Committer:  Ingo Molnar <mingo@...nel.org>
> CommitDate: Sat, 7 May 2016 07:38:39 +0200
> 
> x86/KASLR: Build identity mappings on demand
> 
> Currently KASLR only supports relocation in a small physical range (from
> 16M to 1G), due to using the initial kernel page table identity mapping.
> To support ranges above this, we need to have an identity mapping for the
> desired memory range before we can decompress (and later run) the kernel.

...

> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 8ef1186..f82975b 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -241,6 +241,8 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
>  	 */
>  	mem_avoid[MEM_AVOID_ZO_RANGE].start = input;
>  	mem_avoid[MEM_AVOID_ZO_RANGE].size = (output + init_size) - input;
> +	add_identity_map(mem_avoid[MEM_AVOID_ZO_RANGE].start,
> +			 mem_avoid[MEM_AVOID_ZO_RANGE].size);
>  
>  	/* Avoid initrd. */
>  	initrd_start  = (u64)boot_params->ext_ramdisk_image << 32;
> @@ -249,6 +251,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
>  	initrd_size |= boot_params->hdr.ramdisk_size;
>  	mem_avoid[MEM_AVOID_INITRD].start = initrd_start;
>  	mem_avoid[MEM_AVOID_INITRD].size = initrd_size;
> +	/* No need to set mapping for initrd, it will be handled in VO. */
>  
>  	/* Avoid kernel command line. */
>  	cmd_line  = (u64)boot_params->ext_cmd_line_ptr << 32;
> @@ -259,10 +262,21 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
>  		;
>  	mem_avoid[MEM_AVOID_CMDLINE].start = cmd_line;
>  	mem_avoid[MEM_AVOID_CMDLINE].size = cmd_line_size;
> +	add_identity_map(mem_avoid[MEM_AVOID_CMDLINE].start,
> +			 mem_avoid[MEM_AVOID_CMDLINE].size);
>  
>  	/* Avoid boot parameters. */
>  	mem_avoid[MEM_AVOID_BOOTPARAMS].start = (unsigned long)boot_params;
>  	mem_avoid[MEM_AVOID_BOOTPARAMS].size = sizeof(*boot_params);
> +	add_identity_map(mem_avoid[MEM_AVOID_BOOTPARAMS].start,
> +			 mem_avoid[MEM_AVOID_BOOTPARAMS].size);
> +
> +	/* We don't need to set a mapping for setup_data. */
> +
> +#ifdef CONFIG_X86_VERBOSE_BOOTUP
> +	/* Make sure video RAM can be used. */
> +	add_identity_map(0, PMD_SIZE);
> +#endif
>  }
>  
>  /* Does this memory vector overlap a known avoided area? */
> @@ -421,6 +435,9 @@ unsigned char *choose_random_location(unsigned long input,
>  		goto out;
>  
>  	choice = random_addr;
> +
> +	add_identity_map(choice, output_size);
> +	finalize_identity_maps();

Looks ok except that finalize_identity_maps()'s name is kinda not really
telling me that we're writing CR3 inside and we're thus switching to the
new pagetable. I thought about maybe having something like

	write_cr3(get_identity_map());

to make it really explicit at the callsite that we're loading the
pagetable but you have all that scheme of stubbed functions on 32-bit
and functions doing something on 64-bit.

So how about at least commenting it?

---
From: Borislav Petkov <bp@...e.de>
Date: Sat, 7 May 2016 11:59:40 +0200
Subject: [PATCH] x86/boot: Comment what finalize_identity_maps() does

So it is not really obvious that finalize_identity_maps() doesn't do any
finalization but it *actually* writes CR3 with the ident PGD. Comment
that at the call site.

Signed-off-by: Borislav Petkov <bp@...e.de>
---
 arch/x86/boot/compressed/kaslr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index f82975b0f9d6..f5a138c3fe96 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -437,6 +437,8 @@ unsigned char *choose_random_location(unsigned long input,
 	choice = random_addr;
 
 	add_identity_map(choice, output_size);
+
+	/* This actually loads the identity pagetable on x86_64. */
 	finalize_identity_maps();
 out:
 	return (unsigned char *)choice;
-- 
2.7.3

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.