[PATCH v3] tpm: display warning if using gpio reset with TPM

[Ilias Apalodimas]

On Thu, 16 May 2024 at 02:21, Tim Harvey <tharvey at gateworks.com> 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 at 1 v2.0: VendorID 0x1114, DeviceID 0x3205, RevisionID 0x01 [open]
>  - board with a reset gpio
> u-boot=> tpm init && tpm info
> tpm at 1 v2.0: VendorID 0x1114, DeviceID 0x3205, RevisionID 0x01 [open]
>
> After:
>  - board with no reset gpio
> u-boot=> tpm init && tpm info
> tpm at 1 v2.0: VendorID 0x1114, DeviceID 0x3205, RevisionID 0x01 [open]
>  - board with a reset gpio
> u-boot=> tpm init && tpm info
> tpm at 1: TPM gpio reset should not be used on secure production devices
> tpm at 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 <tharvey at gateworks.com>
> ---
> 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 <ilias.apalodimas at linaro.org>