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]
Date:   Mon,  2 Jul 2018 18:50:53 -0700
From:   Sai Praneeth Prakhya <sai.praneeth.prakhya@...el.com>
To:     linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Sai Praneeth <sai.praneeth.prakhya@...el.com>,
        Lee Chun-Yi <jlee@...e.com>, Dave Young <dyoung@...hat.com>,
        Borislav Petkov <bp@...en8.de>,
        Laszlo Ersek <lersek@...hat.com>,
        Jan Kiszka <jan.kiszka@...mens.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Bhupesh Sharma <bhsharma@...hat.com>,
        Nicolai Stange <nicstange@...il.com>,
        Naresh Bhat <naresh.bhat@...aro.org>,
        Ricardo Neri <ricardo.neri@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Taku Izumi <izumi.taku@...fujitsu.com>,
        Ravi Shankar <ravi.v.shankar@...el.com>,
        Matt Fleming <matt@...eblueprint.co.uk>,
        Dan Williams <dan.j.williams@...el.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>
Subject: [PATCH 5/6] x86/efi: Free allocated memory if remap fails

From: Sai Praneeth <sai.praneeth.prakhya@...el.com>

efi_memmap_alloc(), as the name suggests, allocates memory for a new efi
memory map. It's referenced from couple of places, namely,
efi_arch_mem_reserve() and efi_free_boot_services(). These callers,
after allocating memory, remap it for further use. As usual, a routine
check is performed to confirm successful remap. If the remap fails,
ideally, the allocated memory should be freed but presently we just
return without freeing it up. Hence, fix this bug by freeing the memory
with efi_memmap_free().

Also, efi_fake_memmap() references efi_memmap_alloc() but it frees
memory correctly using memblock_free(), but replace it with
efi_memmap_free() to maintain consistency, as in, allocate memory with
efi_memmap_alloc() and free memory with efi_memmap_free().

It's a fact that memremap() and early_memremap() might never fail and
this code might never get a chance to run but to maintain good kernel
programming semantics, we might need this patch.

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@...el.com>
Suggested-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc: Lee Chun-Yi <jlee@...e.com>
Cc: Dave Young <dyoung@...hat.com>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Laszlo Ersek <lersek@...hat.com>
Cc: Jan Kiszka <jan.kiszka@...mens.com>
Cc: Dave Hansen <dave.hansen@...el.com>
Cc: Bhupesh Sharma <bhsharma@...hat.com>
Cc: Nicolai Stange <nicstange@...il.com>
Cc: Naresh Bhat <naresh.bhat@...aro.org>
Cc: Ricardo Neri <ricardo.neri@...el.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Taku Izumi <izumi.taku@...fujitsu.com>
Cc: Ravi Shankar <ravi.v.shankar@...el.com>
Cc: Matt Fleming <matt@...eblueprint.co.uk>
Cc: Dan Williams <dan.j.williams@...el.com>
Cc: Ard Biesheuvel <ard.biesheuvel@...aro.org>
---
 arch/x86/platform/efi/quirks.c  | 10 ++++++++--
 drivers/firmware/efi/fake_mem.c |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 11800f3cbb93..8fce327387e5 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -286,6 +286,7 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
 	new = early_memremap(new_phys, new_size);
 	if (!new) {
 		pr_err("Failed to map new boot services memmap\n");
+		efi_memmap_free(new_phys, num_entries, alloc_type);
 		return;
 	}
 
@@ -434,7 +435,7 @@ void __init efi_free_boot_services(void)
 	new = memremap(new_phys, new_size, MEMREMAP_WB);
 	if (!new) {
 		pr_err("Failed to map new EFI memmap\n");
-		return;
+		goto free_mem;
 	}
 
 	/*
@@ -460,8 +461,13 @@ void __init efi_free_boot_services(void)
 			efi.memmap.alloc_type);
 	if (efi_memmap_install(new_phys, num_entries, alloc_type)) {
 		pr_err("Could not install new EFI memmap\n");
-		return;
+		goto free_mem;
 	}
+
+	return;
+
+free_mem:
+	efi_memmap_free(new_phys, num_entries, alloc_type);
 }
 
 /*
diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
index a47754efb796..09b0fabf07fd 100644
--- a/drivers/firmware/efi/fake_mem.c
+++ b/drivers/firmware/efi/fake_mem.c
@@ -80,7 +80,7 @@ void __init efi_fake_memmap(void)
 	new_memmap = early_memremap(new_memmap_phy,
 				    efi.memmap.desc_size * new_nr_map);
 	if (!new_memmap) {
-		memblock_free(new_memmap_phy, efi.memmap.desc_size * new_nr_map);
+		efi_memmap_free(new_memmap_phy, new_nr_map, alloc_type);
 		return;
 	}
 
-- 
2.7.4

Powered by blists - more mailing lists