Pltrst lock v2 - #959
Pltrst lock v2#959filipleple wants to merge 8 commits into
Conversation
gpio_lock_pads() is declared to return int, but the success path fell off the end of the function without returning anything. Every caller that looks at the return value, e.g. gpio_lock_pad(), therefore acted on an undefined value on success. Return 0 once all pads have been walked. Upstream-Status: Pending Signed-off-by: Filip Lewiński <filip.lewinski@3mdeb.com>
gpio_lock_pads() always locked the pad configuration with a sideband interface message, while the path used outside of SMM picks between the sideband interface and a private configuration register write based on what the SoC selected. Starting with Meteor Lake, the recommendation is to use PCR writes, so the SMM path silently did nothing there. Pick the same method as gpio_non_smm_lock_pad() does. The PCR helper is moved ahead of its new user to avoid a forward declaration. Upstream-Status: Pending Signed-off-by: Filip Lewiński <filip.lewinski@3mdeb.com>
gpio_lock_pad() sent every caller to gpio_lock_pads() as soon as SOC_INTEL_COMMON_BLOCK_SMM_LOCK_GPIO_PADS was selected, including the callers in gpio_configure_pads(). That function runs in ramstage, so gpio_lock_pads() bailed out with 'must be called from SMM!' and the pads requesting a lock in the mainboard pad tables, i.e. every PAD_CFG_*_LOCK entry, were left unlocked. Route non-SMM callers to gpio_non_smm_lock_pad() regardless of the SMM locking option, so that both ways of locking a pad can be used on the same board. Upstream-Status: Pending Signed-off-by: Filip Lewiński <filip.lewinski@3mdeb.com>
Locking a pad can silently do nothing: PADCFGLOCK writes are only honoured when they carry the SMM security attribute, sideband messages are dropped when the P2SB is unreachable, and the PCR path does not look at the register at all. Callers relying on a pad being locked for security reasons then have no way of telling. Read the lock registers back after locking and report the pads that are still writable, so a failure shows up in the log and in the return value. Upstream-Status: Pending Signed-off-by: Filip Lewiński <filip.lewinski@3mdeb.com>
Tiger Lake never filled in pad_cfg_lock_offset for its GPIO communities and selected no lock method, so gpio_lock_pad() and friends refused to lock any pad on this SoC. Add PAD_CFG_LOCK_OFFSET, 0x80 for PCH-LP and 0x90 for PCH-H, matching Intel document 834810 and the offsets the Linux pinctrl driver uses, and point every community at it. Up to and including Alder Lake PCH the lock registers can only be written through the sideband interface, so select SOC_INTEL_COMMON_BLOCK_GPIO_LOCK_USING_SBI. Upstream-Status: Pending Signed-off-by: Filip Lewiński <filip.lewinski@3mdeb.com>
PLTRST# is the native function of a GPIO pad on Intel PCHs. Software running after the firmware can switch that pad to GPIO mode and drive it, which resets a discrete TPM while the host keeps running: the PCRs go back to their reset values and can be extended with arbitrary measurements, so sealed secrets can be unsealed on a machine that never went through a measured boot. See https://mkukri.xyz/2024/06/01/tpm-gpio-fail.html Add an option for SoCs that provide the pad in soc_gpio_lock_config(), locking it from the SMM finalize handler, which is the only place where the write is honoured and where FSP can no longer undo it. The option depends on INTEL_CHIPSET_LOCKDOWN because that is what makes coreboot send the finalize SMI in the first place. SoCs opt in by selecting SOC_INTEL_COMMON_BLOCK_GPIO_HAS_PLTRST_PAD, so that the option stays hidden where no pad list is provided and the lock would silently do nothing. Upstream-Status: Pending Signed-off-by: Filip Lewiński <filip.lewinski@3mdeb.com>
Provide GPP_B13, the pad driving PLTRST#, to the common code that locks security relevant pads from the SMM finalize handler. Without it, system software can reconfigure the pad as a GPIO and reset a discrete TPM while the host keeps running, clearing the PCRs. Upstream-Status: Pending Signed-off-by: Filip Lewiński <filip.lewinski@3mdeb.com>
Provide GPP_B13, the pad driving PLTRST#, to the common code that locks security relevant pads from the SMM finalize handler. Without it, system software can reconfigure the pad as a GPIO and reset a discrete TPM while the host keeps running, clearing the PCRs. Upstream-Status: Pending Signed-off-by: Filip Lewiński <filip.lewinski@3mdeb.com>
8fcf8a9 to
c816192
Compare
|
@filipleple If i understand correctly, this is not PCR path but SMM SMI lock path? If so, this requires INTEL_CHIPSET_LOCKDOWN=y which Heads doesn't set, deferring to io386 for Heads to call initrd/bin/lock_chip.sh before kexec? Am I missing something? |
@miczyg1 note that none of the patchwork currently kept under dasharo/coreboot for Heads for deferred smm lock per io386 (PR0) is merged upstream https://github.com/linuxboot/heads/blob/master/patches/coreboot-25.09/0003-soc-intel-lockdown-Allow-locking-down-SPI-and-LPC-in.patch -> https://review.coreboot.org/c/coreboot/+/85278 which complicates tracking those dependencies under coreboot upstream. It also slows down other ports needing backporting of the patch, ie chromebooks https://github.com/linuxboot/heads/pull/2133/changes#diff-df80edaef9def0400e30e05f184e6f4d3c297c5fc7de8200f7cf938e230ecc4b per linuxboot/heads#2133 Can we work into merging https://review.coreboot.org/c/coreboot/+/85278 upstream please, that would ease collaboration of all parts interested in this, and in current case, might orient toward PCR approach vs SMM SMI approach, which here depends on |
@filipleple see https://github.com/linuxboot/heads/blob/master/doc/wp-notes.md |
|
Also note that my PoC got updated today at https://github.com/tlaurion/tpm-gpio-fail/ And is packed under all merged TPM2 boards under Heads master per linuxboot/heads#2200 |
|
A Heads-compatible (deferring lockdown by payload) PLTRST# solution should either:
|
|
Ok.... Still SMI/SMM which will not get merged coreboot upstream... Not PCR approach still https://review.coreboot.org/c/coreboot/+/90885 |
|
@tlaurion the PCR method isn't available on ADL/TGL, and it looks like non-SMM locking won't work there. My reasoning for the approach is this:
|
Thanks for those previous explanations @filipleple |
|
@filipleple are you planning to upstream this patch set (or its successor)? If not, I'm happy to if it helps downstreams, just let me know which patch set to start from |
|
@MrChromebox I have so far uploaded the patchset addressing the issue: https://review.coreboot.org/q/topic:%22tpm_gpio_pltrst_lock%22, though I haven't tested an upstream build on hardware yet |
thanks, started reviewing, looks like https://review.coreboot.org/c/coreboot/+/95514 is missing the actual code - it's just the kconfig edit: specifically, there's nothing calling soc_gpio_lock_config() |
|
@MrChromebox thanks, I've amended the commit message:
|
3 participants
No description provided.