Arthur Heymans has uploaded this change for review.
cpu/x86/smm: Add sinkhole mitigation to relocatable smmstub
This adds a check for LAPIC base twice. One very early check when the
CPU is still executing in real mode checks if the LAPIC base is inside
the region [smmbase,smmbase + SMM_DEFAULT_SIZE). This cannot use
anything but a hardcoded size since even accessing the relocatable
parameters is impossible in the state of the CPU.
The actual SMI handler is located above smmbase + SMM_DEFAULT_SIZE and
before jumping to it the LAPIC base is checked against the whole SMM
region. Given that we have a working stack at this point, this is done
in C code.
UNTESTED.
Change-Id: I49927c4f4218552b732bac8aae551d845ad7f079
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
---
M src/cpu/x86/smm/Makefile.inc
A src/cpu/x86/smm/sinkhole.c
M src/cpu/x86/smm/smm_stub.S
M src/include/cpu/x86/smm.h
4 files changed, 87 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/37289/1
diff --git a/src/cpu/x86/smm/Makefile.inc b/src/cpu/x86/smm/Makefile.inc
index 11a4e67..b94f11e 100644
--- a/src/cpu/x86/smm/Makefile.inc
+++ b/src/cpu/x86/smm/Makefile.inc
@@ -45,6 +45,7 @@
postcar-y += tseg_region.c
smmstub-y += smm_stub.S
+smmstub-y += sinkhole.c
smm-y += smm_module_handler.c
diff --git a/src/cpu/x86/smm/sinkhole.c b/src/cpu/x86/smm/sinkhole.c
new file mode 100644
index 0000000..773549c
--- /dev/null
+++ b/src/cpu/x86/smm/sinkhole.c
@@ -0,0 +1,34 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <console/console.h>
+#include <cpu/x86/lapic_def.h>
+#include <cpu/x86/msr.h>
+#include <cpu/x86/smm.h>
+
+const extern struct smm_runtime smm_runtime;
+
+/* Don't try to recover because, if somebody tried to do shenanigans like
+ these, we have to expect more. */
+void mitigate_sinkhole(void)
+{
+ msr_t lapic_base_msr = rdmsr(LAPIC_BASE_MSR);
+ uintptr_t lapic_base = lapic_base_msr.lo & LAPIC_BASE_MSR_ADDR_MASK;
+
+ const uintptr_t smm_end = smm_runtime.smbase + smm_runtime.smm_size;
+
+ if (lapic_base > smm_runtime.smbase && lapic_base < smm_end) {
+ printk(BIOS_EMERG, "Wrong LAPIC base detected! dying...\n");
+ asm("ud2");
+ }
+}
diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S
index 304ea4b..1d39ae1 100644
--- a/src/cpu/x86/smm/smm_stub.S
+++ b/src/cpu/x86/smm/smm_stub.S
@@ -22,6 +22,7 @@
*/
#include <cpu/x86/cr.h>
+#include <cpu/x86/lapic_def.h>
.code32
.section ".module_parameters", "aw", @progbits
@@ -39,6 +40,7 @@
fxsave_area_size:
.long 0
/* struct smm_runtime begins here. */
+.global smm_runtime
smm_runtime:
smbase:
.long 0
@@ -63,10 +65,51 @@
(CR0_CD | CR0_NW | CR0_PG | CR0_AM | CR0_WP | \
CR0_NE | CR0_TS | CR0_EM | CR0_MP)
+#define SMM_DEFAULT_SIZE 0x10000
+
.text
.code16
.global _start
_start:
+#if CONFIG(SMM_LAPIC_REMAP_MITIGATION)
+ /* Check if the LAPIC register block overlaps with SMM.
+ * This block needs to work without data accesses because they
+ * may be routed into the LAPIC register block.
+ * Code accesses, on the other hand, are never routed to LAPIC,
+ * which is what makes this work in the first place.
+ * This is a mitigation against the sinkhole vulnerability
+ * possible on pre sandy bridge Intel hardware.
+ */
+ mov $LAPIC_BASE_MSR, %ecx
+ rdmsr
+ and $(~0xfff), %eax
+ sub $_start, %eax
+ /* This might cover a bit more than needed but in the case that
+ this is a stub to the relocated, permanent handler it should
+ cover some parts of the permanent handler too, so no harm is
+ done. In the case that this is a stub to a relocation handler
+ coreboot did something horribly wrong if LAPIC is here.
+ This has to be 'hardcoded' as the CPU is running in real mode
+ at this point so it cannot access any relocatable variable. */
+ cmp $SMM_DEFAULT_SIZE, %eax
+ ja untampered_lapic
+1:
+ /* emit "Crash" on serial */
+ mov $(CONFIG_TTYS0_BASE), %dx
+ mov $'C', %al
+ out %al, (%dx)
+ mov $'r', %al
+ out %al, (%dx)
+ mov $'a', %al
+ out %al, (%dx)
+ mov $'s', %al
+ out %al, (%dx)
+ mov $'h', %al
+ out %al, (%dx)
+ /* now crash for real */
+ ud2
+untampered_lapic:
+#endif
movl $(smm_relocate_gdt), %ebx
lgdtl (%ebx)
@@ -174,6 +217,12 @@
/* Align stack to 16 bytes. Another 32 bytes are pushed below. */
andl $0xfffffff0, %esp
+#if CONFIG(SMM_LAPIC_REMAP_MITIGATION)
+ /* Now that we have a stack, check for against lapic base in
+ C code before jumping to the permanent handler */
+ call mitigate_sinkhole
+#endif
+
/* Call into the c-based SMM relocation function with the platform
* parameters. Equivalent to:
* struct arg = { c_handler_params, cpu_num, smm_runtime, canary };
diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h
index 2e3c639..884d5cb 100644
--- a/src/include/cpu/x86/smm.h
+++ b/src/include/cpu/x86/smm.h
@@ -89,6 +89,9 @@
/* SMM Runtime helpers. */
+/* SMM Runtime sinkhole mitigation check */
+void mitigate_sinkhole(void);
+
/* Entry point for SMM modules. */
asmlinkage void smm_handler_start(void *params);
To view, visit change 37289. To unsubscribe, or for help writing mail filters, visit settings.