Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1682300
| From | Chao Fan <fanc.fnst@cn.fujitsu.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice |
| Date | 2017-07-06 12:10 +0200 |
| Message-ID | <u0bTs-2TC-3@gated-at.bofh.it> (permalink) |
| References | <u0aun-1Sl-37@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Thu, Jul 06, 2017 at 08:31:07AM +0000, Naoya Horiguchi wrote:
>Hi Baoquan, everyone,
>
>I'm also interested in KASLR/EFI related issue (but not the same issue
>with yours, so I separated the thread.)
>
>This patch is based on Baoquan's recent patches[1], adding more code
>on the new function process_efi_entry().
>If it's OK, could you queue this onto your tree/series?
>
>[1] "[PATCH v3 0/2] x86/boot/KASLR: Restrict kernel to be randomized"
> https://lkml.org/lkml/2017/7/5/98
>
>Thanks,
>Naoya Horiguchi
>---
>From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>Date: Thu, 6 Jul 2017 16:40:52 +0900
>Subject: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from
> KASLR's choice
>
>KASLR chooses kernel location from E820_TYPE_RAM regions by walking over
>e820 entries now. E820_TYPE_RAM includes EFI_BOOT_SERVICES_CODE and
>EFI_BOOT_SERVICES_DATA, so those regions can be the target. According to
>UEFI spec, all memory regions marked as EfiBootServicesCode and
>EfiBootServicesData are available for free memory after the first call
>of ExitBootServices(). So such regions should be usable for kernel on
>spec basis.
>
>In x86, however, we have some workaround for broken firmware, where we
>keep such regions reserved until SetVirtualAddressMap() is done.
>See the following code in should_map_region():
>
> static bool should_map_region(efi_memory_desc_t *md)
> {
> ...
> /*
> * Map boot services regions as a workaround for buggy
> * firmware that accesses them even when they shouldn't.
> *
> * See efi_{reserve,free}_boot_services().
> */
> if (md->type == EFI_BOOT_SERVICES_CODE ||
> md->type == EFI_BOOT_SERVICES_DATA)
> return false;
>
>This workaround suppressed a boot crash, but potential issues still
>remain because no one prevents the regions from overlapping with kernel
>image by KASLR.
>
>So let's make sure that EFI_BOOT_SERVICES_{CODE|DATA} regions are never
>chosen as kernel memory for the workaround to work fine.
>
>Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>---
> arch/x86/boot/compressed/kaslr.c | 41 +++++++++++++++++++++++++++++++---------
> 1 file changed, 32 insertions(+), 9 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>index 94f08fd375ae..f43fed0441a6 100644
>--- a/arch/x86/boot/compressed/kaslr.c
>+++ b/arch/x86/boot/compressed/kaslr.c
>@@ -563,7 +563,8 @@ static void process_mem_region(struct mem_vector *entry,
> /* Marks if efi mirror regions have been found and handled. */
> static bool efi_mirror_found;
>
>-static void process_efi_entry(unsigned long minimum, unsigned long image_size)
>+/* Returns true if we really enter efi memmap walk, otherwise returns false. */
>+static bool process_efi_entry(unsigned long minimum, unsigned long image_size)
> {
> struct efi_info *e = &boot_params->efi_info;
> struct mem_vector region;
>@@ -577,13 +578,13 @@ static void process_efi_entry(unsigned long minimum, unsigned long image_size)
> signature = (char *)&boot_params->efi_info.efi_loader_signature;
> if (strncmp(signature, EFI32_LOADER_SIGNATURE, 4) &&
> strncmp(signature, EFI64_LOADER_SIGNATURE, 4))
>- return;
>+ return false;
>
> #ifdef CONFIG_X86_32
> /* 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;
>+ return false;
> }
> pmap = e->efi_memmap;
> #else
>@@ -593,13 +594,36 @@ static void process_efi_entry(unsigned long minimum, unsigned long image_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->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);
>+ if (md->attribute & EFI_MEMORY_MORE_RELIABLE)
> efi_mirror_found = true;
>+ }
Hi Horiguchi-san,
Sorry for one more suggestion,
How about add:
if (!efi_mirror_found)
return false;
at this place, between the two cycles.
Because if there are no mirror regions found, I think we can return
directly and go to walk the e820 entries.
I don't know whether my understanding is right.
Thanks,
Chao Fan
>+
>+ for (i = 0; i < nr_desc; i++) {
>+ md = (efi_memory_desc_t *)(pmap + (i * e->efi_memdesc_size));
>+
>+ /*
>+ * EFI_BOOT_SERVICES_{CODE|DATA} are avoided because boot
>+ * services regions could be accessed after ExitBootServices()
>+ * due to the workaround for buggy firmware.
>+ */
>+ if (!(md->type == EFI_LOADER_CODE ||
>+ md->type == EFI_LOADER_DATA ||
>+ md->type == EFI_CONVENTIONAL_MEMORY))
>+ continue;
>+
>+ if (efi_mirror_found &&
>+ !(md->attribute & EFI_MEMORY_MORE_RELIABLE))
>+ continue;
>+
>+ region.start = md->phys_addr;
>+ region.size = md->num_pages << EFI_PAGE_SHIFT;
>+ process_mem_region(®ion, minimum, image_size);
>+ if (slot_area_index == MAX_SLOT_AREA) {
>+ debug_putstr("Aborted EFI scan (slot_areas full)!\n");
>+ break;
> }
> }
>+ return true;
> }
>
> static void process_e820_entry(unsigned long minimum, unsigned long image_size)
>@@ -637,8 +661,7 @@ static unsigned long find_random_phys_addr(unsigned long minimum,
> minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN);
>
> #ifdef CONFIG_EFI
>- process_efi_entry(minimum, image_size);
>- if (efi_mirror_found)
>+ if (process_efi_entry(minimum, image_size))
> return slots_fetch_random();
> #endif
>
>--
>2.7.4
>
>
>
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-07-06 10:40 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Chao Fan <fanc.fnst@cn.fujitsu.com> - 2017-07-06 11:20 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-07-06 11:30 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Chao Fan <fanc.fnst@cn.fujitsu.com> - 2017-07-06 11:40 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Baoquan He <bhe@redhat.com> - 2017-07-06 11:20 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-07-06 11:40 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Chao Fan <fanc.fnst@cn.fujitsu.com> - 2017-07-06 12:10 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Chao Fan <fanc.fnst@cn.fujitsu.com> - 2017-07-06 12:30 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Matt Fleming <matt@codeblueprint.co.uk> - 2017-07-06 17:00 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Baoquan He <bhe@redhat.com> - 2017-07-07 05:10 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-07-07 08:20 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Matt Fleming <matt@codeblueprint.co.uk> - 2017-07-07 13:00 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-07-10 07:50 +0200
[PATCH v3 2/2] x86/efi: clean up dead code around efi_reserve_boot_services() Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-07-10 08:00 +0200
[PATCH v3 1/2] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-07-10 08:00 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Matt Fleming <matt@codeblueprint.co.uk> - 2017-07-07 13:00 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Baoquan He <bhe@redhat.com> - 2017-07-09 12:50 +0200
Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Baoquan He <bhe@redhat.com> - 2017-07-09 16:30 +0200
[PATCH v2 2/2] x86/efi: clean up dead code around efi_reserve_boot_services() Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-07-07 09:30 +0200
[PATCH v2 1/2] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-07-07 09:30 +0200
csiph-web