Received-SPF: pass (google.com: domain of linux-kernel+bounces-62926-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 13 Feb 2024 04:07:46 +0000
Precedence: bulk
Mime-Version: 1.0
Message-ID: <20240213040747.1745939-1-kevinloughlin@google.com>
Subject: [PATCH] x86/kernel: Validate ROM before DMI scanning when SEV-SNP is active
From: Kevin Loughlin <kevinloughlin@google.com>
To: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, 
	Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org, 
	"H. Peter Anvin" <hpa@zytor.com>, Baoquan He <bhe@redhat.com>, Josh Poimboeuf <jpoimboe@kernel.org>, 
	"Peter Zijlstra (Intel)" <peterz@infradead.org>, Yuntao Wang <ytcoode@gmail.com>, 
	Kevin Loughlin <kevinloughlin@google.com>, Tom Lendacky <thomas.lendacky@amd.com>, 
	Ard Biesheuvel <ardb@kernel.org>, Dionna Glaze <dionnaglaze@google.com>, 
	Alexander Shishkin <alexander.shishkin@linux.intel.com>, 
	Ross Lagerwall <ross.lagerwall@citrix.com>, Kai Huang <kai.huang@intel.com>, 
	Brijesh Singh <brijesh.singh@amd.com>, linux-kernel@vger.kernel.org
Cc: Adam Dunlap <acdunlap@google.com>, Peter Gonda <pgonda@google.com>, 
	Jacob Xu <jacobhxu@google.com>, Sidharth Telang <sidtelang@google.com>, 
	Conrad Grobler <grobler@google.com>, Andri Saar <andrisaar@google.com>
Content-Type: text/plain; charset="UTF-8"

SEV-SNP requires encrypted memory to be validated before access. The
kernel is responsible for validating the ROM memory range because the
range is not part of the e820 table and therefore not pre-validated by
the BIOS.

While the current SEV-SNP code attempts to validate the ROM range in
probe_roms(), this does not suffice for all existing use cases. In
particular, if EFI_CONFIG_TABLES are not enabled and
CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK is set, the kernel will
attempt to access the memory at SMBIOS_ENTRY_POINT_SCAN_START (which
falls in the ROM range) prior to validation. The specific problematic
call chain occurs during dmi_setup() -> dmi_scan_machine() and results
in a crash during boot if SEV-SNP is enabled under these conditions.

This commit thus provides the simple solution of moving the ROM range
validation from probe_roms() to before dmi_setup(), such that a SEV-SNP
guest satisfying the above use case now successfully boots.

Fixes: 9704c07bf9f7 ("x86/kernel: Validate ROM memory before accessing when SEV-SNP is active")
Signed-off-by: Kevin Loughlin <kevinloughlin@google.com>
---
 arch/x86/include/asm/setup.h |  6 ++++++
 arch/x86/kernel/probe_roms.c | 19 +++++++++----------
 arch/x86/kernel/setup.c      | 10 ++++++++++
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index 5c83729c8e71..5c8f5b0d0f9f 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -117,6 +117,12 @@ void *extend_brk(size_t size, size_t align);
 	__section(".bss..brk") __aligned(1) __used	\
 	static char __brk_##name[size]
 
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+void snp_prep_rom_range(void);
+#else
+static inline void snp_prep_rom_range(void) { }
+#endif
+
 extern void probe_roms(void);
 
 void clear_bss(void);
diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
index 319fef37d9dc..83b192f5e3cc 100644
--- a/arch/x86/kernel/probe_roms.c
+++ b/arch/x86/kernel/probe_roms.c
@@ -196,6 +196,15 @@ static int __init romchecksum(const unsigned char *rom, unsigned long length)
 	return !length && !sum;
 }
 
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+void __init snp_prep_rom_range(void)
+{
+	snp_prep_memory(video_rom_resource.start,
+			((system_rom_resource.end + 1) - video_rom_resource.start),
+			SNP_PAGE_STATE_PRIVATE);
+}
+#endif
+
 void __init probe_roms(void)
 {
 	unsigned long start, length, upper;
@@ -203,16 +212,6 @@ void __init probe_roms(void)
 	unsigned char c;
 	int i;
 
-	/*
-	 * The ROM memory range is not part of the e820 table and is therefore not
-	 * pre-validated by BIOS. The kernel page table maps the ROM region as encrypted
-	 * memory, and SNP requires encrypted memory to be validated before access.
-	 * Do that here.
-	 */
-	snp_prep_memory(video_rom_resource.start,
-			((system_rom_resource.end + 1) - video_rom_resource.start),
-			SNP_PAGE_STATE_PRIVATE);
-
 	/* video rom */
 	upper = adapter_rom_resources[0].start;
 	for (start = video_rom_resource.start; start < upper; start += 2048) {
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 84201071dfac..19f870728486 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -902,6 +902,16 @@ void __init setup_arch(char **cmdline_p)
 		efi_init();
 
 	reserve_ibft_region();
+
+	/*
+	 * The ROM memory range is not part of the e820 table and is therefore not
+	 * pre-validated by BIOS. The kernel page table maps the ROM region as encrypted
+	 * memory, and SNP requires encrypted memory to be validated before access.
+	 * This should be done before dmi_setup(), which may access the ROM region
+	 * even before probe_roms() is called.
+	 */
+	snp_prep_rom_range();
+
 	dmi_setup();
 
 	/*
-- 
2.43.0.687.g38aa6559b0-goog