From: Axel Jonsson <axelj@axis.com>
To: <jarkko@kernel.org>
Cc: <axel.jonsson@axis.com>, <axelj@axis.com>, <borysmn@axis.com>,
<jgg@ziepe.ca>, <kernel@axis.com>,
<linux-integrity@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<peterhuewe@gmx.de>, <sergio.ruiz@axis.com>
Subject: Re: [PATCH v7] tpm: Add Upgrade/Reduced mode support for TPM2 modules
Date: Mon, 13 Dec 2021 08:06:12 +0100 [thread overview]
Message-ID: <20211213070612.3312-1-axelj@axis.com> (raw)
In-Reply-To: <5e5cbdd7db3d8f469f94607b823b8fd46e014051.camel@kernel.org>
>> From: Axel Jonsson <axelj@axis.com>
>>
>> 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 TPM2_FieldUpgradeData(). It may also accept
>> other commands if it is able to complete them using the previously
>> installed firmware.
>> * 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 by adding the following checks. If
>> tpm2_do_selftest() call returns TPM2_RC_UPGRADE, the TPM is in Upgrade
>> mode.
>> 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 detected that the TPM is in the Upgrade or Failure mode, the function
>> sets TPM_CHIP_FLAG_FIRMWARE_UPGRADE_MODE flag.
>>
>> The TPM_CHIP_FLAG_FIRMWARE_UPGRADE_MODE flag is used later during driver
>> initialization/deinitialization to disable functionality which makes no
>> sense or will fail in the current TPM state. Following functionality is
>> affected:
>> * Do not register TPM as a hwrng
>> * Do not register sysfs entries which provide information impossible to
>> obtain in limited mode
>> * Do not register resource managed character device
>>
>> Signed-off-by: Axel Jonsson <axelj@axis.com>
>> ---
>>
>> Notes:
>> Notes:
>> v2:
>> * Commit message updated.
>>
>> v3:
>> * Commit message reworked.
>>
>> v4:
>> * Description of how tpm2_auto_startup() detects the mode added to
>> commit message.
>>
>> v5:
>> * Introduce global flag: TPM_CHIP_FLAG_LIMITED_MODE.
>> * Add checks for the flag in places that will not work properly when TPM
>> functionality is limited.
>> * Avoid registering sysfs and character device entries that have no useful
>> function in limited mode.
>> * Do not register TPM as a hwrng.
>> * Do not try to obtain any crypto-related properties from TPM as it will fail
>> in limited mode.
>>
>> v6:
>> * Rename the TPM_CHIP_FLAG_LIMITED_MODE to TPM_CHIP_FLAG_FIRMWARE_UPGRADE_MODE
>> to reduce confusion. Update info messages.
>>
>> v7:
>> * Clarified in commit message which command can be run in firmware upgrade
>> mode
>> * Corrected info and style in commit message
>> * Renamed TPM_CHIP_FLAG_FIRMWARE_UPGRADE_MODE to TPM_CHIP_FLAG_FIRMWARE_MODE
>> * Replaced TPM_CHIP_FLAG_FIRMWARE_MODE flag check with function from
>> include/linux/tpm.h
>>
>> drivers/char/tpm/tpm-chip.c | 23 ++++++++++++++++-------
>> drivers/char/tpm/tpm-sysfs.c | 3 +++
>> drivers/char/tpm/tpm2-cmd.c | 6 +++++-
>> include/linux/tpm.h | 10 ++++++++++
>> 4 files changed, 34 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index ddaeceb7e109..285d054d187b 100644
>> --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -444,7 +444,8 @@ static int tpm_add_char_device(struct tpm_chip *chip)
>> return rc;
>> }
>>
>> - if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>> + if (chip->flags & TPM_CHIP_FLAG_TPM2 &&
>> + !tpm_is_firmware_upgrade(chip)) {
>
>This can be in a single line (less than 100 characters)?
>
Done.
>> rc = cdev_device_add(&chip->cdevs, &chip->devs);
>> if (rc) {
>> dev_err(&chip->devs,
>> @@ -488,7 +489,8 @@ static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
>> {
>> struct attribute **i;
>>
>> - if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
>> + if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL) ||
>> + tpm_is_firmware_upgrade(chip))
>> return;
>>
>> sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
>> @@ -506,7 +508,8 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
>> struct attribute **i;
>> int rc;
>>
>> - if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
>> + if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL) ||
>> + tpm_is_firmware_upgrade(chip))
>> return 0;
>>
>> rc = compat_only_sysfs_link_entry_to_kobj(
>> @@ -536,7 +539,8 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>>
>> static int tpm_add_hwrng(struct tpm_chip *chip)
>> {
>> - if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM))
>> + if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) ||
>> + tpm_is_firmware_upgrade(chip))
>
>This can be in a single line (less than 100 characters)?
>
Done.
>> return 0;
>>
>> snprintf(chip->hwrng_name, sizeof(chip->hwrng_name),
>> @@ -550,6 +554,9 @@ static int tpm_get_pcr_allocation(struct tpm_chip *chip)
>> {
>> int rc;
>>
>> + if (tpm_is_firmware_upgrade(chip))
>> + return 0;
>> +
>> rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ?
>> tpm2_get_pcr_allocation(chip) :
>> tpm1_get_pcr_allocation(chip);
>> @@ -612,7 +619,7 @@ int tpm_chip_register(struct tpm_chip *chip)
>> return 0;
>>
>> out_hwrng:
>> - if (IS_ENABLED(CONFIG_HW_RANDOM_TPM))
>> + if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) && !tpm_is_firmware_upgrade(chip))
>> hwrng_unregister(&chip->hwrng);
>> out_ppi:
>> tpm_bios_log_teardown(chip);
>> @@ -637,10 +644,12 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
>> void tpm_chip_unregister(struct tpm_chip *chip)
>> {
>> tpm_del_legacy_sysfs(chip);
>> - if (IS_ENABLED(CONFIG_HW_RANDOM_TPM))
>> + if (IS_ENABLED(CONFIG_HW_RANDOM_TPM) &&
>> + !tpm_is_firmware_upgrade(chip))
>
>Ditto.
>
Done.
>> hwrng_unregister(&chip->hwrng);
>> tpm_bios_log_teardown(chip);
>> - if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> + if (chip->flags & TPM_CHIP_FLAG_TPM2 &&
>> + !tpm_is_firmware_upgrade(chip))
>
>Ditto.
>
Done.
>> cdev_device_del(&chip->cdevs, &chip->devs);
>> tpm_del_char_device(chip);
>> }
>> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
>> index 63f03cfb8e6a..54c71473aa29 100644
>> --- a/drivers/char/tpm/tpm-sysfs.c
>> +++ b/drivers/char/tpm/tpm-sysfs.c
>> @@ -480,6 +480,9 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
>>
>> WARN_ON(chip->groups_cnt != 0);
>>
>> + if (tpm_is_firmware_upgrade(chip))
>> + return;
>> +
>> if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
>> else
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index a25815a6f625..24928b0160f1 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -743,8 +743,12 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>> }
>>
>> rc = tpm2_get_cc_attrs_tbl(chip);
>> -
>
>Please, do not remove this empty line.
>
Done.
>> out:
>> + if (rc == TPM2_RC_UPGRADE) {
>> + dev_info(&chip->dev, "TPM in field upgrade mode, requires firmware upgrade\n");
>> + chip->flags |= TPM_CHIP_FLAG_FIRMWARE_MODE;
>> + rc = 0;
>> + }
>
>Empty line here.
>
Done.
>> if (rc > 0)
>> rc = -ENODEV;
>> return rc;
>> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>> index 12d827734686..d38efdae382c 100644
>> --- a/include/linux/tpm.h
>> +++ b/include/linux/tpm.h
>> @@ -207,6 +207,7 @@ enum tpm2_return_codes {
>> TPM2_RC_INITIALIZE = 0x0100, /* RC_VER1 */
>> TPM2_RC_FAILURE = 0x0101,
>> TPM2_RC_DISABLED = 0x0120,
>> + TPM2_RC_UPGRADE = 0x012D,
>> TPM2_RC_COMMAND_CODE = 0x0143,
>> TPM2_RC_TESTING = 0x090A, /* RC_WARN */
>> TPM2_RC_REFERENCE_H0 = 0x0910,
>> @@ -278,6 +279,7 @@ enum tpm_chip_flags {
>> TPM_CHIP_FLAG_HAVE_TIMEOUTS = BIT(4),
>> TPM_CHIP_FLAG_ALWAYS_POWERED = BIT(5),
>> TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED = BIT(6),
>> + TPM_CHIP_FLAG_FIRMWARE_MODE = BIT(7),
>> };
>>
>> #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
>> @@ -399,6 +401,14 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
>> tpm_buf_append(buf, (u8 *) &value2, 4);
>> }
>>
>> +/*
>> + * Check if TPM device is in the firmware upgrade mode.
>> + */
>> +static inline bool tpm_is_firmware_upgrade(struct tpm_chip *chip)
>> +{
>> + return chip->flags & TPM_CHIP_FLAG_FIRMWARE_MODE;
>> +}
>
>For consistency sake, rename the flag as TPM_CHIP_FLAG_FIRMWARE_UPGRADE.
>
Done.
>> +
>> static inline u32 tpm2_rc_value(u32 rc)
>> {
>> return (rc & BIT(7)) ? rc & 0xff : rc;
Best regards, Axel
next prev parent reply other threads:[~2021-12-13 7:06 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-30 16:02 [PATCH v6] tpm: Add Upgrade/Reduced mode support for TPM2 modules Borys Movchan
2021-09-30 16:28 ` Jarkko Sakkinen
2021-09-30 16:31 ` Jarkko Sakkinen
2021-12-08 9:30 ` Axel Jonsson
2021-12-08 9:53 ` [PATCH v7] " Axel Jonsson
2021-12-11 14:15 ` Jarkko Sakkinen
2021-12-13 7:06 ` Axel Jonsson [this message]
2021-12-13 7:09 ` [PATCH v8] " Axel Jonsson
2021-12-21 8:35 ` Jarkko Sakkinen
2021-09-30 16:30 ` [PATCH v6] " Borys Movchan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211213070612.3312-1-axelj@axis.com \
--to=axelj@axis.com \
--cc=axel.jonsson@axis.com \
--cc=borysmn@axis.com \
--cc=jarkko@kernel.org \
--cc=jgg@ziepe.ca \
--cc=kernel@axis.com \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterhuewe@gmx.de \
--cc=sergio.ruiz@axis.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox