On Thu, 16 May 2024 at 02:21, Tim Harvey <[email protected]> wrote: > > Instead of displaying what looks like an error message if a > gpio-reset dt prop is missing for a TPM display a warning that > having a gpio reset on a TPM should not be used for a secure production > device. > > TCG TIS spec [1] says: > "The TPM_Init (LRESET#/SPI_RST#) signal MUST be connected to the > platform CPU Reset signal such that it complies with the requirements > specified in section 1.2.7 HOST Platform Reset in the PC Client > Implementation Specification for Conventional BIOS." > > The reasoning is that you should not be able to toggle a GPIO and reset > the TPM without resetting the CPU as well because if an attacker can > break into your OS via an OS level security flaw they can then reset the > TPM via GPIO and replay the measurements required to unseal keys > that you have otherwise protected. > > Additionally restructure the code for improved readability allowing for > removal of the init label. > > Before: > - board with no reset gpio > u-boot=> tpm init && tpm info > tpm_tis_spi_probe: missing reset GPIO > tpm@1 v2.0: VendorID 0x1114, DeviceID 0x3205, RevisionID 0x01 [open] > - board with a reset gpio > u-boot=> tpm init && tpm info > tpm@1 v2.0: VendorID 0x1114, DeviceID 0x3205, RevisionID 0x01 [open] > > After: > - board with no reset gpio > u-boot=> tpm init && tpm info > tpm@1 v2.0: VendorID 0x1114, DeviceID 0x3205, RevisionID 0x01 [open] > - board with a reset gpio > u-boot=> tpm init && tpm info > tpm@1: TPM gpio reset should not be used on secure production devices > tpm@1 v2.0: VendorID 0x1114, DeviceID 0x3205, RevisionID 0x01 [open] > > [1] > https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientTPMInterfaceSpecification_TIS__1-3_27_03212013.pdf > > Signed-off-by: Tim Harvey <[email protected]> > --- > v3: restructure code for improved readability (recommended by Miquel) > v2: change the message to a warning > --- > drivers/tpm/tpm2_tis_spi.c | 21 ++++++++++++--------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c > index 28079b5039a3..b0fe97ab1d08 100644 > --- a/drivers/tpm/tpm2_tis_spi.c > +++ b/drivers/tpm/tpm2_tis_spi.c > @@ -237,19 +237,22 @@ static int tpm_tis_spi_probe(struct udevice *dev) > /* legacy reset */ > ret = gpio_request_by_name(dev, "gpio-reset", 0, > &reset_gpio, GPIOD_IS_OUT); > - if (ret) { > + if (!ret) { > log(LOGC_NONE, LOGL_NOTICE, > - "%s: missing reset GPIO\n", __func__); > - goto init; > + "%s: gpio-reset is deprecated\n", > __func__); > } > - log(LOGC_NONE, LOGL_NOTICE, > - "%s: gpio-reset is deprecated\n", __func__); > } > - dm_gpio_set_value(&reset_gpio, 1); > - mdelay(1); > - dm_gpio_set_value(&reset_gpio, 0); > + > + if (!ret) { > + log(LOGC_NONE, LOGL_WARNING, > + "%s: TPM gpio reset should not be used on secure > production devices\n", > + dev->name); > + dm_gpio_set_value(&reset_gpio, 1); > + mdelay(1); > + dm_gpio_set_value(&reset_gpio, 0); > + } > } > -init: > + > /* Ensure a minimum amount of time elapsed since reset of the TPM */ > mdelay(drv_data->time_before_first_cmd_ms); > > -- > 2.25.1 > Thanks Tim!
Reviewed-by: Ilias Apalodimas <[email protected]>

