linux-kernel - [PATCH 3.16 44/99] efi/x86: Map the entire EFI vendor string before copying it

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:   Wed, 20 May 2020 15:14:12 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, Denis Kirjanov <kda@...ux-powerpc.org>,
        "Matthew Garrett" <mjg59@...gle.com>,
        "Ard Biesheuvel" <ardb@...nel.org>,
        "Ard Biesheuvel" <ard.biesheuvel@...aro.org>,
        linux-efi@...r.kernel.org, "Arvind Sankar" <nivedita@...m.mit.edu>,
        "Andy Lutomirski" <luto@...nel.org>,
        "Ingo Molnar" <mingo@...nel.org>
Subject: [PATCH 3.16 44/99] efi/x86: Map the entire EFI vendor string
 before copying it

3.16.84-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Ard Biesheuvel <ardb@...nel.org>

commit ffc2760bcf2dba0dbef74013ed73eea8310cc52c upstream.

Fix a couple of issues with the way we map and copy the vendor string:
- we map only 2 bytes, which usually works since you get at least a
  page, but if the vendor string happens to cross a page boundary,
  a crash will result
- only call early_memunmap() if early_memremap() succeeded, or we will
  call it with a NULL address which it doesn't like,
- while at it, switch to early_memremap_ro(), and array indexing rather
  than pointer dereferencing to read the CHAR16 characters.

Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
Cc: Andy Lutomirski <luto@...nel.org>
Cc: Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc: Arvind Sankar <nivedita@...m.mit.edu>
Cc: Matthew Garrett <mjg59@...gle.com>
Cc: linux-efi@...r.kernel.org
Fixes: 5b83683f32b1 ("x86: EFI runtime service support")
Link: https://lkml.kernel.org/r/20200103113953.9571-5-ardb@kernel.org
Signed-off-by: Ingo Molnar <mingo@...nel.org>
[bwh: Backported to 3.16: Keep using early_memremap() since
 early_memremap_ro() is not defined.]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 arch/x86/platform/efi/efi.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -718,7 +718,6 @@ void __init efi_init(void)
 	efi_char16_t *c16;
 	char vendor[100] = "unknown";
 	int i = 0;
-	void *tmp;
 
 #ifdef CONFIG_X86_32
 	if (boot_params.efi_info.efi_systab_hi ||
@@ -745,14 +744,16 @@ void __init efi_init(void)
 	/*
 	 * Show what we know for posterity
 	 */
-	c16 = tmp = early_memremap(efi.systab->fw_vendor, 2);
+	c16 = early_memremap(efi.systab->fw_vendor,
+			     sizeof(vendor) * sizeof(efi_char16_t));
 	if (c16) {
-		for (i = 0; i < sizeof(vendor) - 1 && *c16; ++i)
-			vendor[i] = *c16++;
+		for (i = 0; i < sizeof(vendor) - 1 && c16[i]; ++i)
+			vendor[i] = c16[i];
 		vendor[i] = '\0';
-	} else
+		early_memunmap(c16, sizeof(vendor) * sizeof(efi_char16_t));
+	} else {
 		pr_err("Could not map the firmware vendor!\n");
-	early_memunmap(tmp, 2);
+	}
 
 	pr_info("EFI v%u.%.02u by %s\n",
 		efi.systab->hdr.revision >> 16,