linux-kernel - [PATCH v3 1/2] x86/boot/KASLR: exclude EFI_BOOT_SERVICES

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1499665896-23731-1-git-send-email-n-horiguchi@ah.jp.nec.com>
Date:   Mon, 10 Jul 2017 14:51:35 +0900
From:   Naoya Horiguchi <n-horiguchi@...jp.nec.com>
To:     Naoya Horiguchi <n-horiguchi@...jp.nec.com>
Cc:     Baoquan He <bhe@...hat.com>, Kees Cook <keescook@...omium.org>,
        linux-kernel@...r.kernel.org, <x86@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...nel.org>,
        <izumi.taku@...fujitsu.com>, Thomas Garnier <thgarnie@...gle.com>,
        <fanc.fnst@...fujitsu.com>,
        Matt Fleming <matt@...eblueprint.co.uk>,
        "Jun'ichi Nomura" <j-nomura@...jp.nec.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>
Subject: [PATCH v3 1/2] 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@...jp.nec.com>
---
v2 -> v3:
- skip EFI_LOADER_CODE and EFI_LOADER_DATA in region scan

v1 -> v2:
- switch efi_mirror_found to local variable
- insert break when EFI_MEMORY_MORE_RELIABLE found
---
 arch/x86/boot/compressed/kaslr.c | 44 +++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git next-20170705/arch/x86/boot/compressed/kaslr.c next-20170705_patched/arch/x86/boot/compressed/kaslr.c
index 94f08fd..44778e9 100644
--- next-20170705/arch/x86/boot/compressed/kaslr.c
+++ next-20170705_patched/arch/x86/boot/compressed/kaslr.c
@@ -560,10 +560,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;
@@ -572,18 +570,18 @@ static void process_efi_entry(unsigned long minimum, unsigned long image_size)
 	char *signature;
 	u32 nr_desc;
 	int i;
-
+	bool efi_mirror_found;
 
 	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
@@ -594,12 +592,35 @@ static void process_efi_entry(unsigned long minimum, unsigned long image_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(&region, minimum, image_size);
 			efi_mirror_found = true;
+			break;
 		}
 	}
+
+	for (i = 0; i < nr_desc; i++) {
+		md = (efi_memory_desc_t *)(pmap + (i * e->efi_memdesc_size));
+
+		/*
+		 * According to spec, EFI_BOOT_SERVICES_{CODE|DATA} are also
+		 * available for kernel image, but we don't include them for
+		 * the workaround for buggy firmware.
+		 */
+		if (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(&region, 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 +658,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.0