Messages in this thread Patch in this message | | | Date | Wed, 23 Aug 2017 16:24:25 +0800 | From | Baoquan He <> | 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@ah.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@ah.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 > >
| |