linux-kernel - [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code

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-next>] [day] [month] [year] [list]

Date:   Thu, 4 Apr 2019 20:26:55 +0000
From:   "Hook, Gary" <Gary.Hook@....com>
To:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>, "luto@...nel.org" <luto@...nel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>
Subject: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early
 boot code

Enablement of AMD's Secure Memory Encryption feature is determined
very early in the boot cycle. Part of this procedure involves scanning
the command line for the paramater 'mem_encrypt'.

To determine intended state, the function sme_enable() uses library
functions cmdline_find_option() and strncmp().  Their use occurs early
enough such that we can't assume that any instrumentation subsystem is
initialized. For example, making calls to a KASAN-instrumented
function before KASAN is set up will likely result in the use of
uninitialized memory and a boot failure.

Avoid instrumenting these dependent functions by:

1) Making a local, static, renamed copy of strncpy() for use solely in
mem_encrypt_identity.c. In this file we are able to vet its few uses
and avoid exposing the rest of the kernel to a ubiquitously used but
un-instrumented function.

2) Disable instrumention of arch/x86/lib/cmdline.c based on the
assumption that the needed function (cmdline_find_option()) is vetted
through its use to date, and contains no lurking flaws that have not
yet been found through instrumentation such as KASAN.

Fixes: aca20d546214  ("x86/mm: Add support to make use of Secure Memory Encryption")
Reported-by: Li RongQing <lirongqing@...du.com>
Signed-off-by: Gary R Hook <gary.hook@....com>
---
 arch/x86/lib/Makefile              |   13 +++++++++++++
 arch/x86/mm/mem_encrypt_identity.c |   26 ++++++++++++++++++++++++--
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 140e61843a07..38182a64fa81 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -6,6 +6,19 @@
 # Produces uninteresting flaky coverage.
 KCOV_INSTRUMENT_delay.o	:= n
 
+# SME early boot code checks the cmdline, so don't instrument
+KCOV_INSTRUMENT_cmdline.o              := n
+
+KASAN_SANITIZE_cmdline.o               := n
+
+ifdef CONFIG_FUNCTION_TRACER
+CFLAGS_REMOVE_cmdline.o                        = -pg
+endif
+
+# No stack protector
+nostackp := $(call cc-option, -fno-stack-protector)
+CFLAGS_cmdline.o               := $(nostackp)
+
 inat_tables_script = $(srctree)/arch/x86/tools/gen-insn-attr-x86.awk
 inat_tables_maps = $(srctree)/arch/x86/lib/x86-opcode-map.txt
 quiet_cmd_inat_tables = GEN     $@
diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
index 4aa9b1480866..0a68d7ccb371 100644
--- a/arch/x86/mm/mem_encrypt_identity.c
+++ b/arch/x86/mm/mem_encrypt_identity.c
@@ -77,6 +77,28 @@ static char sme_cmdline_arg[] __initdata = "mem_encrypt";
 static char sme_cmdline_on[]  __initdata = "on";
 static char sme_cmdline_off[] __initdata = "off";
 
+/*
+ * Local copy to avoid instrumentation
+ * Copied from lib/string.c and renamed to be unique.
+ * This file is early boot code, and we assume that instrumentation
+ * subsystems (e.g. KASAN) are not yet initialized.
+ */
+static int sme_strncmp(const char *cs, const char *ct, size_t count)
+{
+	unsigned char c1, c2;
+
+	while (count) {
+		c1 = *cs++;
+		c2 = *ct++;
+		if (c1 != c2)
+			return c1 < c2 ? -1 : 1;
+		if (!c1)
+			break;
+		count--;
+	}
+	return 0;
+}
+
 static void __init sme_clear_pgd(struct sme_populate_pgd_data *ppd)
 {
 	unsigned long pgd_start, pgd_end, pgd_size;
@@ -557,9 +579,9 @@ void __init sme_enable(struct boot_params *bp)
 
 	cmdline_find_option(cmdline_ptr, cmdline_arg, buffer, sizeof(buffer));
 
-	if (!strncmp(buffer, cmdline_on, sizeof(buffer)))
+	if (!sme_strncmp(buffer, cmdline_on, sizeof(buffer)))
 		sme_me_mask = me_mask;
-	else if (!strncmp(buffer, cmdline_off, sizeof(buffer)))
+	else if (!sme_strncmp(buffer, cmdline_off, sizeof(buffer)))
 		sme_me_mask = 0;
 	else
 		sme_me_mask = active_by_default ? me_mask : 0;