Patrick Rudolph submitted this change.
security/intel: Add option to enable SMM flash access only
On platforms where the boot media can be updated externally, e.g.
using a BMC, add the possibility to enable writes in SMM only. This
allows to protect the BIOS region even without the use of vboot, but
keeps SMMSTORE working for use in payloads. Note that this breaks
flashconsole, since the flash becomes read-only.
Tested on Asrock B85M Pro4 and HP 280 G2, SMM BIOS write protection
works as expected, and SMMSTORE can still be used.
Change-Id: I157db885b5f1d0f74009ede6fb2342b20d9429fa
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40830
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
---
M src/security/lockdown/Kconfig
M src/soc/intel/common/block/fast_spi/fast_spi.c
M src/soc/intel/common/block/include/intelblocks/fast_spi.h
M src/soc/intel/common/block/smm/smihandler.c
M src/soc/intel/common/pch/lockdown/lockdown.c
M src/southbridge/intel/common/finalize.c
M src/southbridge/intel/common/spi.c
7 files changed, 95 insertions(+), 35 deletions(-)
diff --git a/src/security/lockdown/Kconfig b/src/security/lockdown/Kconfig
index 9d83a45..8d48beb 100644
--- a/src/security/lockdown/Kconfig
+++ b/src/security/lockdown/Kconfig
@@ -84,6 +84,17 @@
ramstage, like the MRC cache for example.
Use this option if you don't trust code running after verstage.
+config BOOTMEDIA_SMM_BWP
+ bool "Boot media only writable in SMM"
+ depends on !CONSOLE_SPI_FLASH
+ depends on BOOT_DEVICE_SPI_FLASH && HAVE_SMI_HANDLER
+ depends on SOUTHBRIDGE_INTEL_COMMON_SPI || SOC_INTEL_COMMON_BLOCK_SPI
+ select SOC_INTEL_COMMON_BLOCK_SMM_TCO_ENABLE if SOC_INTEL_COMMON_BLOCK_SPI
+ help
+ Only allow flash writes in SMM. Select this if you want to use SMMSTORE
+ while also preventing unauthorized writes through the internal controller.
+ Note that this breaks flashconsole, since the flash becomes read-only.
+
choice
prompt "SPI Flash write protection duration"
default BOOTMEDIA_SPI_LOCK_REBOOT
diff --git a/src/soc/intel/common/block/fast_spi/fast_spi.c b/src/soc/intel/common/block/fast_spi/fast_spi.c
index 15cd26e..843071e 100644
--- a/src/soc/intel/common/block/fast_spi/fast_spi.c
+++ b/src/soc/intel/common/block/fast_spi/fast_spi.c
@@ -418,3 +418,14 @@
bios_cntl &= ~SPI_BIOS_CONTROL_WPD;
pci_write_config8(dev, SPI_BIOS_CONTROL, bios_cntl);
}
+
+/* Disable SPI Write Protect. */
+void fast_spi_disable_wp(void)
+{
+ const pci_devfn_t dev = PCH_DEV_SPI;
+ uint8_t bios_cntl;
+
+ bios_cntl = pci_read_config8(dev, SPI_BIOS_CONTROL);
+ bios_cntl |= SPI_BIOS_CONTROL_WPD;
+ pci_write_config8(dev, SPI_BIOS_CONTROL, bios_cntl);
+}
diff --git a/src/soc/intel/common/block/include/intelblocks/fast_spi.h b/src/soc/intel/common/block/include/intelblocks/fast_spi.h
index d044256..d5d3cd5 100644
--- a/src/soc/intel/common/block/include/intelblocks/fast_spi.h
+++ b/src/soc/intel/common/block/include/intelblocks/fast_spi.h
@@ -77,6 +77,10 @@
*/
void fast_spi_enable_wp(void);
/*
+ * Disable SPI Write protect.
+ */
+void fast_spi_disable_wp(void);
+/*
* Get base and size of extended BIOS decode window used at runtime in host address space. If
* the BIOS region is not greater than 16MiB, then this function returns 0 for both base and
* size.
diff --git a/src/soc/intel/common/block/smm/smihandler.c b/src/soc/intel/common/block/smm/smihandler.c
index 91142a5..5789b48 100644
--- a/src/soc/intel/common/block/smm/smihandler.c
+++ b/src/soc/intel/common/block/smm/smihandler.c
@@ -44,31 +44,6 @@
return 1;
}
-/*
- * Needs to implement the mechanism to know if an illegal attempt
- * has been made to write to the BIOS area.
- */
-static void smihandler_soc_check_illegal_access(
- uint32_t tco_sts)
-{
- if (!((tco_sts & (1 << 8)) && CONFIG(SPI_FLASH_SMM)
- && fast_spi_wpd_status()))
- return;
-
- /*
- * BWE is RW, so the SMI was caused by a
- * write to BWE, not by a write to the BIOS
- *
- * This is the place where we notice someone
- * is trying to tinker with the BIOS. We are
- * trying to be nice and just ignore it. A more
- * resolute answer would be to power down the
- * box.
- */
- printk(BIOS_DEBUG, "Switching back to RO\n");
- fast_spi_enable_wp();
-}
-
/* Mainboard overrides. */
__weak void mainboard_smi_gpi_handler(
@@ -301,9 +276,19 @@
/* Parameter buffer in EBX */
reg_ebx = save_state_ops->get_reg(io_smi, RBX);
+ const bool wp_enabled = !fast_spi_wpd_status();
+ if (wp_enabled) {
+ fast_spi_disable_wp();
+ /* Not clearing SPI sync SMI status here results in hangs */
+ fast_spi_clear_sync_smi_status();
+ }
+
/* drivers/smmstore/smi.c */
ret = smmstore_exec(sub_command, (void *)(uintptr_t)reg_ebx);
save_state_ops->set_reg(io_smi, RAX, ret);
+
+ if (wp_enabled)
+ fast_spi_enable_wp();
}
static void finalize(void)
@@ -320,6 +305,9 @@
/* Re-init SPI driver to handle locked BAR */
fast_spi_init();
+ if (CONFIG(BOOTMEDIA_SMM_BWP))
+ fast_spi_enable_wp();
+
/*
* HECI is disabled in smihandler_soc_at_finalize() which also locks down the side band
* interface. Some boards may require this interface in mainboard_smi_finalize(),
@@ -401,12 +389,26 @@
*/
fast_spi_clear_sync_smi_status();
+ /* If enabled, enforce SMM BIOS write protection */
+ if (CONFIG(BOOTMEDIA_SMM_BWP) && fast_spi_wpd_status()) {
+ /*
+ * BWE is RW, so the SMI was caused by a
+ * write to BWE, not by a write to the BIOS
+ *
+ * This is the place where we notice someone
+ * is trying to tinker with the BIOS. We are
+ * trying to be nice and just ignore it. A more
+ * resolute answer would be to power down the
+ * box.
+ */
+ printk(BIOS_DEBUG, "Switching SPI back to RO\n");
+ fast_spi_enable_wp();
+ }
+
/* Any TCO event? */
if (!tco_sts)
return;
- smihandler_soc_check_illegal_access(tco_sts);
-
if (tco_sts & TCO_TIMEOUT) { /* TIMEOUT */
/* Handle TCO timeout */
printk(BIOS_DEBUG, "TCO Timeout.\n");
diff --git a/src/soc/intel/common/pch/lockdown/lockdown.c b/src/soc/intel/common/pch/lockdown/lockdown.c
index d9495a4..87f36fc 100644
--- a/src/soc/intel/common/pch/lockdown/lockdown.c
+++ b/src/soc/intel/common/pch/lockdown/lockdown.c
@@ -65,6 +65,12 @@
/* BIOS Interface Lock */
fast_spi_set_bios_interface_lock_down();
+ /* Only allow writes in SMM */
+ if (CONFIG(BOOTMEDIA_SMM_BWP)) {
+ //fast_spi_set_eiss(); /* TODO */
+ fast_spi_enable_wp();
+ }
+
/* BIOS Lock */
fast_spi_set_lock_enable();
diff --git a/src/southbridge/intel/common/finalize.c b/src/southbridge/intel/common/finalize.c
index 975d839..6fb27bb 100644
--- a/src/southbridge/intel/common/finalize.c
+++ b/src/southbridge/intel/common/finalize.c
@@ -45,6 +45,9 @@
pci_write_config32(PCI_DEV(0, 27, 0), 0x74,
pci_read_config32(PCI_DEV(0, 27, 0), 0x74));
+ if (CONFIG(BOOTMEDIA_SMM_BWP))
+ write_pmbase16(SMI_EN, read_pmbase16(SMI_EN) | TCO_EN);
+
write_pmbase16(TCO1_CNT, read_pmbase16(TCO1_CNT) | TCO_LOCK);
post_code(POST_OS_BOOT);
diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c
index 828f3d3..30f7657 100644
--- a/src/southbridge/intel/common/spi.c
+++ b/src/southbridge/intel/common/spi.c
@@ -27,6 +27,8 @@
static int spi_is_multichip(void);
+static void spi_set_smm_only_flashing(bool enable);
+
struct ich7_spi_regs {
uint16_t spis;
uint16_t spic;
@@ -282,7 +284,6 @@
void spi_init(void)
{
- uint8_t bios_cntl;
struct ich9_spi_regs *ich9_spi;
struct ich7_spi_regs *ich7_spi;
uint16_t hsfs;
@@ -332,13 +333,8 @@
ich_set_bbar(0);
- if (CONFIG(SOUTHBRIDGE_INTEL_I82801GX) || CONFIG(SOUTHBRIDGE_INTEL_COMMON_SPI_ICH9)) {
- /* Disable the BIOS write protect so write commands are allowed. */
- bios_cntl = pci_read_config8(dev, 0xdc);
- /* Deassert SMM BIOS Write Protect Disable. */
- bios_cntl &= ~(1 << 5);
- pci_write_config8(dev, 0xdc, bios_cntl | 0x1);
- }
+ /* Disable the BIOS write protect so write commands are allowed. */
+ spi_set_smm_only_flashing(false);
}
static int spi_locked(void)
@@ -1096,12 +1092,39 @@
writeb_(spi_config->ops[i].op, &cntlr.opmenu[i]);
}
writew_(optype, cntlr.optype);
+
+ spi_set_smm_only_flashing(CONFIG(BOOTMEDIA_SMM_BWP));
}
__weak void intel_southbridge_override_spi(struct intel_swseq_spi_config *spi_config)
{
}
+#define BIOS_CNTL 0xdc
+#define BIOS_CNTL_BIOSWE (1 << 0)
+#define BIOS_CNTL_BLE (1 << 1)
+#define BIOS_CNTL_SMM_BWP (1 << 5)
+
+static void spi_set_smm_only_flashing(bool enable)
+{
+ if (!(CONFIG(SOUTHBRIDGE_INTEL_I82801GX) || CONFIG(SOUTHBRIDGE_INTEL_COMMON_SPI_ICH9)))
+ return;
+
+ const pci_devfn_t dev = PCI_DEV(0, 31, 0);
+
+ uint8_t bios_cntl = pci_read_config8(dev, BIOS_CNTL);
+
+ if (enable) {
+ bios_cntl &= ~BIOS_CNTL_BIOSWE;
+ bios_cntl |= BIOS_CNTL_BLE | BIOS_CNTL_SMM_BWP;
+ } else {
+ bios_cntl &= ~(BIOS_CNTL_BLE | BIOS_CNTL_SMM_BWP);
+ bios_cntl |= BIOS_CNTL_BIOSWE;
+ }
+
+ pci_write_config8(dev, BIOS_CNTL, bios_cntl);
+}
+
static const struct spi_ctrlr spi_ctrlr = {
.xfer_vector = xfer_vectors,
.max_xfer_size = member_size(struct ich9_spi_regs, fdata),
To view, visit change 40830. To unsubscribe, or for help writing mail filters, visit settings.