| 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
| ||
|
Date: Wed, 23 Aug 2017 16:24:25 +0800
From: Baoquan He <bhe@...hat.com>
To: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
Cc: Matt Fleming <matt@...eblueprint.co.uk>,
Kees Cook <keescook@...omium.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"x86@...nel.org" <x86@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...nel.org>,
"izumi.taku@...fujitsu.com" <izumi.taku@...fujitsu.com>,
Thomas Garnier <thgarnie@...gle.com>,
"fanc.fnst@...fujitsu.com" <fanc.fnst@...fujitsu.com>,
Junichi Nomura <j-nomura@...jp.nec.com>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
"dyoung@...hat.com" <dyoung@...hat.com>
Subject: Re: [PATCH] x86/boot: check overlap between kernel and
EFI_BOOT_SERVICES_*
Hi Naoya,
On 07/28/17 at 06:48am, Naoya Horiguchi wrote:
> > > > So I think of adding some assertion in the patch 1/2 to detect this overlap
> > > > in extract_kernel() even for no KASLR case.
> > >
> > > EFI_BOOT_SERVICES_* memory are collected as e820 region of
> > > E820_TYPE_RAM, how can we guarantee kernel won't use them after jumping
> > > into the running kernel whether KASLR enabled or not? We can only wish
>
> Current kernels have no such check, so it's not guarantted, I think.
> And I guess that most firmwares (luckily?) avoid the overlap, and/or
> if the overlap happens, that might not cause any detectable error.
>
> > > that EFI firmware engineer don't put EFI_BOOT_SERVICES_* far from
> > sorry, typo. I meant EFI boot
> > service region need be put far from 0x1000000. Otherwise normal kernel could
> > allocate memory bottom up and stomp on them. It's embarassment caused by
> > the hardware flaw of x86 platfrom.
> > > 0x1000000 where normal kernel is loaded.
>
>
> > > > So I think of adding some assertion in the patch 1/2 to detect this overlap
> > > > in extract_kernel() even for no KASLR case.
>
> Anyway I wrote the following patch adding the assertion as a separate one.
> I'm glad if I can get your feedback or advise.
Sorry, I didn't notice your mail in this deep thread. I think it makes
sense to do something about the overlap. Maybe you can repost a patch
independently, or put it together with the patchset excluding
EFI_BOOT_SERVICES_{CODE|DATA}. Let's see what x86 maintainers and EFI
subsystem maintainers will say.
By the way, Ingo has helped to put my patchset of kernel mirror handling
related to kaslr into tip/x86/boot, you can base your
EFI_BOOT_SERVICES_{CODE|DATA} excluding patchset and repost. Maybe start
a new thread, it might not be easy to follow in a too deep thread.
THanks
Baoquan
> From: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
> Date: Thu, 27 Jul 2017 13:19:35 +0900
> Subject: [PATCH] x86/boot: check overlap between kernel and
> EFI_BOOT_SERVICES_*
>
> Even when KASLR is turned off, kernel still could overlap with
> EFI_BOOT_SERVICES_* region, for example because of firmware memmap
> layout and/or CONFIG_PHYSICAL_START.
>
> So this patch adds an assertion that we are free from the overlap
> which is called for non-KASLR case.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
> ---
> arch/x86/boot/compressed/kaslr.c | 3 ---
> arch/x86/boot/compressed/misc.c | 56 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index a6604717d4fe..5549c80b45c2 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -721,9 +721,6 @@ void choose_random_location(unsigned long input,
>
> boot_params->hdr.loadflags |= KASLR_FLAG;
>
> - /* Prepare to add new identity pagetables on demand. */
> - initialize_identity_maps();
> -
> /* Record the various known unsafe memory ranges. */
> mem_avoid_init(input, input_size, *output);
>
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index a0838ab929f2..b23159fa159c 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -15,6 +15,8 @@
> #include "error.h"
> #include "../string.h"
> #include "../voffset.h"
> +#include <linux/efi.h>
> +#include <asm/efi.h>
>
> /*
> * WARNING!!
> @@ -169,6 +171,55 @@ void __puthex(unsigned long value)
> }
> }
>
> +#ifdef CONFIG_EFI
> +bool __init
> +efi_kernel_boot_services_overlap(unsigned long start, unsigned long size)
> +{
> + int i;
> + u32 nr_desc;
> + struct efi_info *e = &boot_params->efi_info;
> + efi_memory_desc_t *md;
> + char *signature;
> + unsigned long pmap;
> +
> + signature = (char *)&boot_params->efi_info.efi_loader_signature;
> + if (strncmp(signature, EFI32_LOADER_SIGNATURE, 4) &&
> + strncmp(signature, EFI64_LOADER_SIGNATURE, 4))
> + return false;
> +
> +#ifdef CONFIG_X86 /* Can't handle data above 4GB at this time */
> + if (e->efi_memmap_hi) {
> + warn("Memory map is above 4GB, EFI should be disabled.\n");
> + return false;
> + }
> + pmap = e->efi_memmap;
> +#else
> + pmap = (e->efi_memmap | ((__u64)e->efi_memmap_hi << 32));
> +#endif
> +
> + add_identity_map(pmap, e->efi_memmap_size);
> +
> + nr_desc = e->efi_memmap_size / e->efi_memdesc_size;
> + for (i = 0; i < nr_desc; i++) {
> + md = (efi_memory_desc_t *)(pmap + (i * e->efi_memdesc_size));
> + if (md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) <= start)
> + continue;
> + if (md->phys_addr >= start + size)
> + continue;
> + if (md->type == EFI_BOOT_SERVICES_CODE ||
> + md->type == EFI_BOOT_SERVICES_DATA)
> + return true;
> + }
> + return false;
> +}
> +#else
> +bool __init
> +efi_kernel_boot_services_overlap(unsigned long start, unsigned long size)
> +{
> + return false;
> +}
> +#endif
> +
> #if CONFIG_X86_NEED_RELOCS
> static void handle_relocations(void *output, unsigned long output_len,
> unsigned long virt_addr)
> @@ -372,6 +423,9 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
> debug_putaddr(output_len);
> debug_putaddr(kernel_total_size);
>
> + /* Prepare to add new identity pagetables on demand. */
> + initialize_identity_maps();
> +
> /*
> * The memory hole needed for the kernel is the larger of either
> * the entire decompressed kernel plus relocation table, or the
> @@ -402,6 +456,8 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
> if (virt_addr != LOAD_PHYSICAL_ADDR)
> error("Destination virtual address changed when not relocatable");
> #endif
> + if (efi_kernel_boot_services_overlap((unsigned long)output, output_len))
> + error("Kernel overlaps EFI_BOOT_SERVICES area");
>
> debug_putstr("\nDecompressing Linux... ");
> __decompress(input_data, input_len, NULL, NULL, output, output_len,
> --
> 2.7.4
>
>
Powered by blists - more mailing lists