Re: [PATCH v4] tpm: Add Upgrade/Reduced mode support for TPM2 modules

[Borys Movchan]

On 8/9/21 6:55 AM, Jarkko Sakkinen wrote:
> On Fri, Aug 06, 2021 at 04:18:08PM +0200, Borys Movchan wrote:
>> If something went wrong during the TPM firmware upgrade, like power
>> failure or the firmware image file get corrupted, the TPM might end
>> up in Upgrade or Failure mode upon the next start. The state is
>> persistent between the TPM power cycle/restart.
>>
>> According to TPM specification:
>>  * If the TPM is in Upgrade mode, it will answer with TPM2_RC_UPGRADE
>>    to all commands except Field Upgrade related ones.
>>  * If the TPM is in Failure mode, it will allow performing TPM
>>    initialization but will not provide any crypto operations.
>>    Will happily respond to Field Upgrade calls.
>>
>> Change the behavior of the tpm2_auto_startup(), so it detects the active
>> running mode of the TPM.  It is easy to determine that TPM is in Upgrade
>> mode by relying on the fact that tpm2_do_selftest() will return
>> TPM2_RC_UPGRADE. In such a case, there is no point to finish the
>> start-up procedure as the TPM will not accept any commands, except
>> firmware upgrade related.
>>
>> On the other hand, if the TPM is in Failure mode, it will successfully
>> respond to both tpm2_do_selftest() and tpm2_startup() calls. Although,
>> will fail to answer to tpm2_get_cc_attrs_tbl(). Use this fact to
>> conclude that TPM is in Failure mode.
>>
>> If the chip is in the Upgrade or Failure mode, the function returns -EIO
>> error code.
>>
>> The return value is checked in the tpm_chip_register() call to determine
>> the state of the TPM. If the TPM is not in normal operation mode, set
>> the `limited_mode` flag. If the flag is set then the TPM is not able to
> Nit: do not use hyphens for limited mode. 'limited_mode' is fine. I'm
> also fine with just limited_mode.


Done


>> provide any crypto functionality.  Correspondignly, the calls to
>> tpm2_get_cc_attrs_tbl(), tpm_add_hwrng() and tpm_get_pcr_allocation()
>> will fail. Use the flag to exclude them from the initialization
>> sequence.
> This is blacklisting. E.g. I'm not sure why all of the sysfs attributes
> would still be exported. Some of them use TPM commands. That was just
> one random example I came up with.
>
> It's easy to come up other examples, like, why you provide still tpmrm0,
> which is dependent on a TPM running normal mode?
>
> This misses completely the rationale for ever acking this change: which
> parts of the uapi are export and *why*.
>
> Please whitelist the things that should still work. Even the obvious
> ones like /dev/tpm0 (because of TPM_RC_UPGRADE).
>
> This is clearly a faulty and incomplete patch in its current form.


I expected something like this. In new patch version I tried to disable all
functionality which should not work in Upgrade/Failure mode.

Basically, I am interested in getting /dev/tpm0 working when TPM is in limited mode.
In this state, the TPM is not capable to provide any other functionality except firmware
upgrade/recovery. So the rest of features should be disabled/removed.
Physical reset of the TPM is mandatory part of the firmware upgrade/recovery. Which will
lead to driver rebind/reload and reappearance of all interfaces applicable to normal operational
mode.


> /Jarkko
>

Kind regards,
Borys