SMM: Validate more user-provided pointers

Mitigate issues presented in "Digging Into The Core of Boot" found by
"Yuriy Bulygin" and "Oleksandr Bazhaniuk" at RECON-MTL-2017.

Validate user-provided pointers using the newly-added functions.
This protects SMM from ring0 attacks.

Change-Id: I8a347ccdd20816924bf1bceb3b24bf7b22309312
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Christian Walter <christian.walter@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41086
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>

8 files changed, 58 insertions, 14 deletions

diff --git a/src/mainboard/lenovo/t60/smihandler.c b/src/mainboard/lenovo/t60/smihandler.c
index fe732a32b5..69ffe33209 100644
--- a/src/mainboard/lenovo/t60/smihandler.c
+++ b/src/mainboard/lenovo/t60/smihandler.c
@@ -2,6 +2,7 @@
 
 #include <arch/io.h>
 #include <device/pci_ops.h>
+#include <device/pci_def.h>
 #include <console/console.h>
 #include <cpu/x86/smm.h>
 #include <southbridge/intel/i82801gx/nvs.h>
@@ -16,24 +17,32 @@
 
 static void mainboard_smi_brightness_down(void)
 {
-	u8 *bar;
-	if ((bar = (u8 *)pci_read_config32(PCI_DEV(1, 0, 0), 0x18))) {
-		printk(BIOS_DEBUG, "bar: %08X, level %02X\n",  (unsigned int)bar, *(bar+LVTMA_BL_MOD_LEVEL));
-		*(bar+LVTMA_BL_MOD_LEVEL) &= 0xf0;
-		if (*(bar+LVTMA_BL_MOD_LEVEL) > 0x10)
-			*(bar+LVTMA_BL_MOD_LEVEL) -= 0x10;
-	}
+	uint32_t reg32 = pci_read_config32(PCI_DEV(1, 0, 0), PCI_BASE_ADDRESS_2) & ~0xf;
+	u8 *bar = (void *)(uintptr_t)reg32;
+
+	/* Validate pointer before using it */
+	if (!bar || smm_points_to_smram(bar, LVTMA_BL_MOD_LEVEL + sizeof(uint8_t)))
+		return;
+
+	printk(BIOS_DEBUG, "bar: %p, level %02X\n", bar, *(bar+LVTMA_BL_MOD_LEVEL));
+	*(bar+LVTMA_BL_MOD_LEVEL) &= 0xf0;
+	if (*(bar+LVTMA_BL_MOD_LEVEL) > 0x10)
+		*(bar+LVTMA_BL_MOD_LEVEL) -= 0x10;
 }
 
 static void mainboard_smi_brightness_up(void)
 {
-	u8 *bar;
-	if ((bar = (u8 *)pci_read_config32(PCI_DEV(1, 0, 0), 0x18))) {
-		printk(BIOS_DEBUG, "bar: %08X, level %02X\n",  (unsigned int)bar, *(bar+LVTMA_BL_MOD_LEVEL));
-		*(bar+LVTMA_BL_MOD_LEVEL) |= 0x0f;
-		if (*(bar+LVTMA_BL_MOD_LEVEL) < 0xf0)
-			*(bar+LVTMA_BL_MOD_LEVEL) += 0x10;
-	}
+	uint32_t reg32 = pci_read_config32(PCI_DEV(1, 0, 0), PCI_BASE_ADDRESS_2) & ~0xf;
+	u8 *bar = (void *)(uintptr_t)reg32;
+
+	/* Validate pointer before using it */
+	if (!bar || smm_points_to_smram(bar, LVTMA_BL_MOD_LEVEL + sizeof(uint8_t)))
+		return;
+
+	printk(BIOS_DEBUG, "bar: %p, level %02X\n", bar, *(bar+LVTMA_BL_MOD_LEVEL));
+	*(bar+LVTMA_BL_MOD_LEVEL) |= 0x0f;
+	if (*(bar+LVTMA_BL_MOD_LEVEL) < 0xf0)
+		*(bar+LVTMA_BL_MOD_LEVEL) += 0x10;
 }
 
 int mainboard_io_trap_handler(int smif)
diff --git a/src/soc/intel/baytrail/smihandler.c b/src/soc/intel/baytrail/smihandler.c
index 6f3f07e73d..1810821ec8 100644
--- a/src/soc/intel/baytrail/smihandler.c
+++ b/src/soc/intel/baytrail/smihandler.c
@@ -321,6 +321,10 @@ static void southbridge_smi_apmc(void)
 		if (state) {
 			/* EBX in the state save contains the GNVS pointer */
 			gnvs = (struct global_nvs *)((uint32_t)state->rbx);
+			if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+				printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
+				return;
+			}
 			smm_initialized = 1;
 			printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
 		}
diff --git a/src/soc/intel/braswell/smihandler.c b/src/soc/intel/braswell/smihandler.c
index a2c26c119f..d2f73bf654 100644
--- a/src/soc/intel/braswell/smihandler.c
+++ b/src/soc/intel/braswell/smihandler.c
@@ -301,6 +301,10 @@ static void southbridge_smi_apmc(void)
 		if (state) {
 			/* EBX in the state save contains the GNVS pointer */
 			gnvs = (struct global_nvs *)((uint32_t)state->rbx);
+			if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+				printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
+				return;
+			}
 			smm_initialized = 1;
 			printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
 		}
diff --git a/src/soc/intel/broadwell/smihandler.c b/src/soc/intel/broadwell/smihandler.c
index 86be400e71..8dbb40f2b7 100644
--- a/src/soc/intel/broadwell/smihandler.c
+++ b/src/soc/intel/broadwell/smihandler.c
@@ -100,6 +100,10 @@ static void backlight_off(void)
 	reg_base = (void *)((uintptr_t)pci_read_config32(SA_DEV_IGD,
 		PCI_BASE_ADDRESS_0) & ~0xf);
 
+	/* Validate pointer before using it */
+	if (smm_points_to_smram(reg_base, PCH_PP_OFF_DELAYS + sizeof(uint32_t)))
+		return;
+
 	/* Check if backlight is enabled */
 	pp_ctrl = read32(reg_base + PCH_PP_CONTROL);
 	if (!(pp_ctrl & EDP_BLC_ENABLE))
@@ -341,6 +345,10 @@ static void southbridge_smi_apmc(void)
 		if (state) {
 			/* EBX in the state save contains the GNVS pointer */
 			gnvs = (struct global_nvs *)((u32)state->rbx);
+			if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+				printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
+				return;
+			}
 			smm_initialized = 1;
 			printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
 		}
diff --git a/src/soc/intel/common/block/smm/smihandler.c b/src/soc/intel/common/block/smm/smihandler.c
index 7bd17c3e78..4998532837 100644
--- a/src/soc/intel/common/block/smm/smihandler.c
+++ b/src/soc/intel/common/block/smm/smihandler.c
@@ -373,6 +373,10 @@ void smihandler_southbridge_apmc(
 			/* EBX in the state save contains the GNVS pointer */
 			uint32_t reg_ebx = save_state_ops->get_reg(state, RBX);
 			gnvs = (struct global_nvs *)(uintptr_t)reg_ebx;
+			if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+				printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
+				return;
+			}
 			smm_initialized = 1;
 			printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
 		}
diff --git a/src/southbridge/intel/bd82x6x/smihandler.c b/src/southbridge/intel/bd82x6x/smihandler.c
index 8af14283c3..7211da37a9 100644
--- a/src/southbridge/intel/bd82x6x/smihandler.c
+++ b/src/southbridge/intel/bd82x6x/smihandler.c
@@ -4,6 +4,7 @@
 #include <arch/io.h>
 #include <device/pci_ops.h>
 #include <console/console.h>
+#include <commonlib/region.h>
 #include <device/pci_def.h>
 #include <cpu/x86/smm.h>
 #include <cpu/intel/em64t101_save_state.h>
@@ -103,6 +104,7 @@ static void xhci_sleep(u8 slp_typ)
 
 		xhci_bar = pci_read_config32(PCH_XHCI_DEV, PCI_BASE_ADDRESS_0) & ~0xFUL;
 
+		/* FIXME: This looks broken (conditions are always false) */
 		if ((xhci_bar + 0x4C0) & 1)
 			pch_iobp_update(0xEC000082, ~0UL, (3 << 2));
 		if ((xhci_bar + 0x4D0) & 1)
@@ -191,6 +193,11 @@ void southbridge_update_gnvs(u8 apm_cnt, int *smm_done)
 	if (state) {
 		/* EBX in the state save contains the GNVS pointer */
 		gnvs = (struct global_nvs *)((u32)state->rbx);
+		struct region r = {(uintptr_t)gnvs, sizeof(struct global_nvs)};
+		if (smm_region_overlaps_handler(&r)) {
+			printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
+			return;
+		}
 		*smm_done = 1;
 		printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
 	}
diff --git a/src/southbridge/intel/ibexpeak/smihandler.c b/src/southbridge/intel/ibexpeak/smihandler.c
index 2bc31cf0cf..6c3f349ce3 100644
--- a/src/southbridge/intel/ibexpeak/smihandler.c
+++ b/src/southbridge/intel/ibexpeak/smihandler.c
@@ -152,6 +152,10 @@ void southbridge_update_gnvs(u8 apm_cnt, int *smm_done)
 	if (state) {
 		/* EBX in the state save contains the GNVS pointer */
 		gnvs = (struct global_nvs *)((u32)state->rbx);
+		if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+			printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
+			return;
+		}
 		*smm_done = 1;
 		printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
 	}
diff --git a/src/southbridge/intel/lynxpoint/smihandler.c b/src/southbridge/intel/lynxpoint/smihandler.c
index bb05f99439..5ccb229805 100644
--- a/src/southbridge/intel/lynxpoint/smihandler.c
+++ b/src/southbridge/intel/lynxpoint/smihandler.c
@@ -314,6 +314,10 @@ static void southbridge_smi_apmc(void)
 		if (state) {
 			/* EBX in the state save contains the GNVS pointer */
 			gnvs = (struct global_nvs *)((u32)state->rbx);
+			if (smm_points_to_smram(gnvs, sizeof(*gnvs))) {
+				printk(BIOS_ERR, "SMI#: ERROR: GNVS overlaps SMM\n");
+				return;
+			}
 			smm_initialized = 1;
 			printk(BIOS_DEBUG, "SMI#: Setting GNVS to %p\n", gnvs);
 		}