Messages in this thread Patch in this message | | | From | Naoya Horiguchi <> | Subject | [PATCH] x86/boot: check overlap between kernel and EFI_BOOT_SERVICES_* | Date | Fri, 28 Jul 2017 06:48:10 +0000 |
| |
On Wed, Jul 26, 2017 at 09:34:32AM +0800, Baoquan He wrote: > On 07/26/17 at 09:13am, Baoquan He wrote: > > On 07/26/17 at 12:12am, Naoya Horiguchi wrote: > > > On Mon, Jul 24, 2017 at 02:20:44PM +0100, Matt Fleming wrote: > > > > On Mon, 10 Jul, at 02:51:36PM, Naoya Horiguchi wrote: > > > > > EFI_BOOT_SERVICES_{CODE|DATA} regions never overlap the kernel now, > > > > > so we can clean up the check in efi_reserve_boot_services(). > > > > > > > > > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > > > > > --- > > > > > arch/x86/platform/efi/quirks.c | 23 +---------------------- > > > > > 1 file changed, 1 insertion(+), 22 deletions(-) > > > > > > > > Is this true for kernels not using KASLR? > > > > > > Thank you for pointing out this. It's not true depending on memmap layout. > > > If a firmware does not define the memory around the kernel address > > > (0x1000000 or CONFIG_PHYSICAL_START) as EFI_BOOT_SERVICES_*, no overlap > > > happens. That's true in my testing server, but I don't think that we can > > > expect it generally. > > > > > > 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.
Thanks, Naoya Horiguchi --- 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
| |