linux-kernel - [PATCH 5/6] x86/efi: Free allocated memory if remap fails

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: <1530582654-25662-6-git-send-email-sai.praneeth.prakhya@intel.com>
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