Felix Held submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
cpu/x86/smm: Add sinkhole mitigation to relocatable smmstub

The sinkhole exploit exists in placing the lapic base such that it
messes with GDT. This can be mitigated by checking the lapic MSR
against the current program counter.

Change-Id: I49927c4f4218552b732bac8aae551d845ad7f079
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37289
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
M src/cpu/x86/smm/smm_stub.S
1 file changed, 39 insertions(+), 0 deletions(-)

diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S
index c83839c..02532a4 100644
--- a/src/cpu/x86/smm/smm_stub.S
+++ b/src/cpu/x86/smm/smm_stub.S
@@ -45,10 +45,49 @@
(CR0_CD | CR0_NW | CR0_PG | CR0_AM | CR0_WP | \
CR0_NE | CR0_TS | CR0_EM | CR0_MP)

+#define SMM_DEFAULT_SIZE 0x10000
.global _start
+ /* Check if the LAPIC register block overlaps with the stub.
+ * 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.
+ */
+ mov $LAPIC_BASE_MSR, %ecx
+ rdmsr
+ and $(~0xfff), %eax
+ call 1f
+ /* Get the current program counter */
+ pop %ebx
+ sub %ebx, %eax
+ cmp $(SMM_DEFAULT_SIZE), %eax
+ ja untampered_lapic
+ /* 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
movl $(smm_relocate_gdt), %ebx
lgdtl (%ebx)

To view, visit change 37289. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I49927c4f4218552b732bac8aae551d845ad7f079
Gerrit-Change-Number: 37289
Gerrit-PatchSet: 8
Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-CC: Paul Menzel <paulepanter@mailbox.org>
Gerrit-MessageType: merged