Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752226AbdHRPKo (ORCPT ); Fri, 18 Aug 2017 11:10:44 -0400 Received: from mail-it0-f46.google.com ([209.85.214.46]:36757 "EHLO mail-it0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752077AbdHRPKl (ORCPT ); Fri, 18 Aug 2017 11:10:41 -0400 MIME-Version: 1.0 In-Reply-To: <20170817130405.GE20203@x1> References: <1502722464-20614-1-git-send-email-bhe@redhat.com> <1502722464-20614-3-git-send-email-bhe@redhat.com> <20170817130405.GE20203@x1> From: Ard Biesheuvel Date: Fri, 18 Aug 2017 16:10:40 +0100 Message-ID: Subject: Re: [PATCH v9 2/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions To: Baoquan He Cc: Ingo Molnar , "linux-kernel@vger.kernel.org" , Kees Cook , "tglx@linutronix.de" , "hpa@zytor.com" , Taku Izumi , fanc.fnst@cn.fujitsu.com, Thomas Garnier , n-horiguchi@ah.jp.nec.com, "linux-efi@vger.kernel.org" , "x86@kernel.org" , Matt Fleming Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5775 Lines: 150 On 17 August 2017 at 14:04, Baoquan He wrote: > On 08/14/17 at 10:54pm, Baoquan He wrote: >> Currently KASLR will parse all e820 entries of RAM type and add all >> candidate position into slots array. Then we will choose one slot >> randomly as the new position which kernel will be decompressed into >> and run at. >> >> On system with EFI enabled, e820 memory regions are coming from EFI >> memory regions by combining adjacent regions. While these EFI memory >> regions have more attributes to mark their different use. Mirror >> attribute is such kind. The physical memory region whose descriptors >> in EFI memory map has EFI_MEMORY_MORE_RELIABLE attribute (bit: 16) are >> mirrored. The address range mirroring feature of kernel arranges such >> mirror region into normal zone and other region into movable zone. And >> with mirroring feature enabled, the code and date of kernel can only be >> located in more reliable mirror region. However, the current KASLR code >> doesn't check EFI memory entries, and could choose new position in >> non-mirrored region. This will break the functionality of the address >> range mirroring feature. > > Thanks a lot for helping improving the patch log, Ingo! Will pay more > attention to the description in words and paragraph partition of log. > >> >> So if EFI is detected, iterate EFI memory map and pick the mirror region >> to process for adding candidate of randomization slot. If EFI is disabled >> or no mirror region found, still process e820 memory map. >> >> Signed-off-by: Baoquan He Could the x86 people on cc either take these directly, or indicate whether they are ok with this going in via the EFI tree? Thanks. >> --- >> arch/x86/boot/compressed/kaslr.c | 68 ++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 66 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c >> index 99c7194f7ea6..7de23bb279ce 100644 >> --- a/arch/x86/boot/compressed/kaslr.c >> +++ b/arch/x86/boot/compressed/kaslr.c >> @@ -37,7 +37,9 @@ >> #include >> #include >> #include >> +#include >> #include >> +#include >> >> /* Macros used by the included decompressor code below. */ >> #define STATIC >> @@ -558,6 +560,65 @@ static void process_mem_region(struct mem_vector *entry, >> } >> } >> >> +#ifdef CONFIG_EFI >> +/* >> + * Returns true if mirror region found (and must have been processed >> + * for slots adding) >> + */ >> +static bool >> +process_efi_entries(unsigned long minimum, unsigned long image_size) >> +{ >> + struct efi_info *e = &boot_params->efi_info; >> + bool efi_mirror_found = false; >> + struct mem_vector region; >> + efi_memory_desc_t *md; >> + unsigned long pmap; >> + char *signature; >> + u32 nr_desc; >> + int i; >> + >> + signature = (char *)&e->efi_loader_signature; >> + if (strncmp(signature, EFI32_LOADER_SIGNATURE, 4) && >> + strncmp(signature, EFI64_LOADER_SIGNATURE, 4)) >> + return false; >> + >> +#ifdef CONFIG_X86_32 >> + /* Can't handle data above 4GB at this time */ >> + if (e->efi_memmap_hi) { >> + warn("EFI memmap is above 4GB, can't be handled now on x86_32. EFI should be disabled.\n"); >> + return false; >> + } >> + pmap = e->efi_memmap; >> +#else >> + pmap = (e->efi_memmap | ((__u64)e->efi_memmap_hi << 32)); >> +#endif >> + >> + nr_desc = e->efi_memmap_size / e->efi_memdesc_size; >> + for (i = 0; i < nr_desc; i++) { >> + md = efi_early_memdesc_ptr(pmap, e->efi_memdesc_size, i); >> + if (md->attribute & EFI_MEMORY_MORE_RELIABLE) { >> + region.start = md->phys_addr; >> + region.size = md->num_pages << EFI_PAGE_SHIFT; >> + process_mem_region(®ion, minimum, image_size); >> + efi_mirror_found = true; >> + >> + if (slot_area_index == MAX_SLOT_AREA) { >> + debug_putstr("Aborted EFI scan (slot_areas full)!\n"); >> + break; >> + } >> + } >> + } >> + >> + return efi_mirror_found; >> +} >> +#else >> +static inline bool >> +process_efi_entries(unsigned long minimum, unsigned long image_size) >> +{ >> + return false; >> +} >> +#endif >> + >> static void process_e820_entries(unsigned long minimum, >> unsigned long image_size) >> { >> @@ -586,13 +647,16 @@ static unsigned long find_random_phys_addr(unsigned long minimum, >> { >> /* Check if we had too many memmaps. */ >> if (memmap_too_large) { >> - debug_putstr("Aborted e820 scan (more than 4 memmap= args)!\n"); >> + debug_putstr("Aborted memory entries scan (more than 4 memmap= args)!\n"); >> return 0; >> } >> >> /* Make sure minimum is aligned. */ >> minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN); >> >> + if (process_efi_entries(minimum, image_size)) >> + return slots_fetch_random(); >> + >> process_e820_entries(minimum, image_size); >> return slots_fetch_random(); >> } >> @@ -652,7 +716,7 @@ void choose_random_location(unsigned long input, >> */ >> min_addr = min(*output, 512UL << 20); >> >> - /* Walk e820 and find a random address. */ >> + /* Walk available memory entries to find a random address. */ >> random_addr = find_random_phys_addr(min_addr, output_size); >> if (!random_addr) { >> warn("Physical KASLR disabled: no suitable memory region!"); >> -- >> 2.5.5 >>