Re: [PATCH v3] tpm: Issue a TPM2_Shutdown for TPM2devices.

[Josh Zimmerman <joshz-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]

This is a bit hard to track down, but I think I've found a relevant
bit of the PTP spec (section 3.8):
https://www.trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2-0-v43-150126.pdf

> The TPM2_Shutdown
> (STATE) command allows a Static OS to indicate to the TPM that the platform may
> enter a low power state where the TPM will be required to enter into the D3 power
> state. The use of the term "may" is significant in that there is no requirement for the
> platform to actually enter the low power state after sending the TPM2_Shutdown
> (STATE) command. The software may, in fact, send subsequent commands after
> sending the TPM2_Shutdown (STATE) commands. The TPM2_Shutdown (STATE)
> command simply tells the TPM to save the required volatile contents because power to
> the TPM may be removed at any time. The TPM is responsible for tracking its internal
> state so that, if a command that alters the TPM’s saved state is sent to the TPM after a
> TPM2_Shutdown (STATE) command, the TPM voids the saved internal state so a
> subsequent TPM2_Startup(STATE) will fail. In this case, it is the responsibility of
> platform software to send a subsequent TPM2_Shutdown (STATE) command to
> preserve the new internal state of the TPM.

From Part 1 (architecture),
https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-1-Architecture-01.38.pdf:

> A TPM implementation may invalidate a preserved context on any command except TPM2_GetCapability().

I haven't found anything in the spec that explicitly addresses being
able to immediately follow a TPM2_Shutdown(STATE) with a
TPM2_Shutdown(CLEAR), but my understanding based on the first quote is
that a TPM2_Shutdown(CLEAR) may clear the previous shutdown state, and
as long as the Shutdown(CLEAR) is the final command, the shutdown will
be orderly.
Josh


On Thu, May 18, 2017 at 2:13 PM, Stefan Berger <stefanb@us.ibm.com> wrote:
> Does it work when doing suspend (to RAM) and tpm_pm_suspend sent a
> tpm2_shutdown(chip, TPM2_SU_STATE) presumably before that?
>
>     Stefan
>
>
> ----- Original message -----
> From: Josh Zimmerman <joshz@google.com>
> To: Peter Huewe <peterhuewe@gmx.de>, Marcel Selhorst <tpmdd@selhorst.net>,
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>, Jason Gunthorpe
> <jgunthorpe@obsidianresearch.com>, tpmdd-devel@lists.sourceforge.net
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Subject: Re: [tpmdd-devel] [PATCH v3] tpm: Issue a TPM2_Shutdown for TPM2
> devices.
> Date: Thu, May 18, 2017 11:22 AM
>
> Are there any other changes I should make to this patch, or is it good
> to go once the patch it depends on is in?
>
> Thanks!
> Josh
>
>
> On Mon, May 15, 2017 at 5:08 PM, Josh Zimmerman <joshz@google.com> wrote:
>> If a TPM2 loses power without a TPM2_Shutdown command being issued (a
>> "disorderly reboot"), it may lose some state that has yet to be
>> persisted to NVRam, and will increment the DA counter (eventually, this
>> will cause the TPM to lock the user out.)
>>
>> NOTE: This only changes behavior on TPM2 devices. Since TPM1 uses sysfs,
>> and sysfs relies on implicit locking on chip->ops, it is not safe to
>> allow this code to run in TPM1, or to add sysfs support to TPM2, until
>> that locking is made explicit.
>>
>> This patch is dependent on '[PATCH] Add "shutdown" to "struct class".'
>> http://marc.info/?l=linux-kernel&m=149463235025420&w=2
>>
>> Signed-off-by: Josh Zimmerman <joshz@google.com>
>>
>> ----
>> v2:
>>   - Properly split changes between this and another commit
>>   - Use proper locking primitive.
>>   - Fix commenting style
>> v3:
>>   - Re-fix commenting style
>> ---
>>  drivers/char/tpm/tpm-chip.c  | 20 ++++++++++++++++++++
>>  drivers/char/tpm/tpm-sysfs.c |  3 +++
>>  2 files changed, 23 insertions(+)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index 9dec9f551b83..272a42e77574 100644
>> --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -142,6 +142,25 @@ static void tpm_devs_release(struct device *dev)
>>         put_device(&chip->dev);
>>  }
>>
>> +static void tpm_shutdown(struct device *dev)
>> +{
>> +       struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
>> +       /* TPM 2.0 requires that the TPM2_Shutdown() command be issued
>> prior to
>> +        * loss of power. If it is not, the DA counter will be incremented
>> and,
>> +        * eventually, the user will be locked out of their TPM.
>> +        * XXX: This codepath relies on the fact that sysfs is not enabled
>> for
>> +        * TPM2: sysfs uses an implicit lock on chip->ops, so this use
>> could
>> +        * race if TPM2 has sysfs support enabled before TPM sysfs's
>> implicit
>> +        * locking is fixed.
>> +        */
>> +       if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>> +               down_write(&chip->ops_sem);
>> +               tpm2_shutdown(chip, TPM_SU_CLEAR);
>> +               chip->ops = NULL;
>> +               up_write(&chip->ops_sem);
>> +       }
>> +}
>> +
>>  /**
>>   * tpm_chip_alloc() - allocate a new struct tpm_chip instance
>>   * @pdev: device to which the chip is associated
>> @@ -181,6 +200,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>>         device_initialize(&chip->devs);
>>
>>         chip->dev.class = tpm_class;
>> +       chip->dev.class.shutdown = tpm_shutdown;
>>         chip->dev.release = tpm_dev_release;
>>         chip->dev.parent = pdev;
>>         chip->dev.groups = chip->groups;
>> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
>> index 55405dbe43fa..5e5ff7eb6f7e 100644
>> --- a/drivers/char/tpm/tpm-sysfs.c
>> +++ b/drivers/char/tpm/tpm-sysfs.c
>> @@ -294,6 +294,9 @@ static const struct attribute_group tpm_dev_group = {
>>
>>  void tpm_sysfs_add_device(struct tpm_chip *chip)
>>  {
>> +       /* XXX: Before this restriction is removed, tpm_sysfs must be
>> updated
>> +        * to explicitly lock chip->ops.
>> +        */
>>         if (chip->flags & TPM_CHIP_FLAG_TPM2)
>>                 return;
>>
>> --
>> 2.13.0.303.g4ebf302169-goog
>>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
>
>
>
>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel