[PATCH] x86/boot: check overlap between kernel and EFI_BOOT_SERVICES_*
From: Naoya Horiguchi
Date: Fri Jul 28 2017 - 02:50:14 EST
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@xxxxxxxxxxxxx>
> > > > > ---
> > > > > 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@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>
---
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