From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752765AbcEGKFv (ORCPT ); Sat, 7 May 2016 06:05:51 -0400 Received: from mail.skyhub.de ([78.46.96.112]:37581 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750936AbcEGKFt (ORCPT ); Sat, 7 May 2016 06:05:49 -0400 Date: Sat, 7 May 2016 12:05:41 +0200 From: Borislav Petkov To: Kees Cook , Ingo Molnar Cc: bp@suse.de, jkosina@suse.cz, linux-kernel@vger.kernel.org, keescook@chromium.org, brgerst@gmail.com, vgoyal@redhat.com, akpm@linux-foundation.org, tglx@linutronix.de, bhe@redhat.com, luto@kernel.org, dyoung@redhat.com, yinghai@kernel.org, dvlasenk@redhat.com, mingo@kernel.org, peterz@infradead.org, luto@amacapital.net, torvalds@linux-foundation.org, hpa@zytor.com, linux-tip-commits@vger.kernel.org Subject: Re: [tip:x86/boot] x86/KASLR: Build identity mappings on demand Message-ID: <20160507100541.GA24613@pd.tnic> References: <1462572095-11754-4-git-send-email-keescook@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > AuthorDate: Fri, 6 May 2016 15:01:35 -0700 > Committer: Ingo Molnar > 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 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 --- 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.