Bug #576
openGPIO locking is broken on Kaby Lake and possibly other platforms
0%
Description
Many supported Kaby Lake boards (and possibly newer platforms as well) are vulnerable to TPM GPIO reset attacks.
Trying to fix this by marking the affected GPIOs as locked in gpio.h and even also selecting SOC_INTEL_COMMON_BLOCK_SMM_LOCK_GPIO_PADS does not work.
This was discovered last year and briefly discussed on #coreboot, but it came up again on the Heads matrix group in relation to supporting the TPM on the in-progress ThinkPad T480 port.
NC Updated by Nicholas Chin 24 days ago ยท Edited
I've looked through the code and this is what I've found so far:
There are two basic functions for locking GPIO pads, gpio_lock_pads() and gpio_non_smm_lock_pad(), both in soc/intel/common/block/gpio/gpio.c.
gpio_lock_pads() can only be called from SMM, and requires that SOC_INTEL_COMMON_BLOCK_SMM_LOCK_GPIO_PADS is selected. There are two possible code paths where this function is invoked:
- The finalize SMI handler (
soc/intel/common/block/gpio/gpio.c:finalize()), which callssoc_lock_gpios(). This requires that the platform implement asoc_gpio_lock_config()override to return which GPIOs should be locked, which no platform implements. Thus, locking from the SMI handler does not work even ifSOC_INTEL_COMMON_BLOCK_SMM_LOCK_GPIO_PADSis selected. -
soc/intel/common/block/gpio/gpio.c:gpio_lock_pad(), which callsgpio_lock_pads()if called from SMM code. This does not suffer from thesoc_gpio_lock_config()described above, but this is almost always called from ramstage through thegpio_configure_pads()>gpio_configure_pad()call stack. The one exception ismb/google/dedede, which calls it from a mainboard SMI handler. It will also callgpio_lock_pads()ifSOC_INTEL_COMMON_BLOCK_SMM_LOCK_GPIO_PADSis selected, even if called from ramstage, but this will also fail to lock the pads asgpio_lock_pads()checks that it is called from SMM.
Thus, the SMM method doesn't work because essentially no platform implements it.
gpio_non_smm_lock_pad() is only called from gpio_lock_pad() if outside of SMM (almost always true as mentioned above) and if SOC_INTEL_COMMON_BLOCK_SMM_LOCK_GPIO_PADS is NOT selected. For this function to perform the lock, either SOC_INTEL_COMMON_BLOCK_GPIO_LOCK_USING_PCR or SOC_INTEL_COMMON_BLOCK_GPIO_LOCK_USING_SBI must be selected, which choose the method to access the GPIO registers in the Private Configuration Space. The former uses the MMIO mapping of the registers through SBREG, while the latter sends sideband messages through the P2SB registers. Currently, only Alderlake onwards selects these configs, so GPIO locking using this method is expected to be broken on all soc/intel platforms prior to this that use the common GPIO code.
The latter method was added in commit fe678cbd195d ("soc/intel/common/gpio: Perform GPIO PAD lock outside SMM").
Commit 222852a26476 ("soc/intel/gpio: Update GPIO Lock configuration recommendation") seems to suggest that platforms prior to Alderlake can only set the lock bits using the SBI method, though I have yet to determine if this is true. The datasheets seem to suggest that the PCR method should work, which I would prefer as it consists of low overhead MMIO accesses.
One other aspect that needs to be looked into is how the lockdown bits are enforced. According to PCH datasheets, after a lock bit is changed from 0 (unlocked) to 1 (locked), attempts to change it back to the unlocked state will signal an SMI. This may imply that SMM is responsible to change the bit back to 1, similar to how the old BIOS Lock Enable bit in the BIOS Control register was intended to work when setting the BIOS Write Enable bit. That method often led to security issues due to no SMI handler being implemented to change the write enable bit back to disabled.
NC Updated by Nicholas Chin 24 days ago
Possible fix here:
https://review.coreboot.org/c/coreboot/+/90884
https://review.coreboot.org/c/coreboot/+/90885
Still needs testing and verification, and extension to other affected platforms. It seems that the SBI method doesn't work on Skylake, though the PCR method does.