[v5,7/7] tpm: pass an array of tpm_bank_list structures to tpm_pcr_extend()
diff mbox series

Message ID 20181114153108.12907-8-roberto.sassu@huawei.com
State New
Headers show
Series
  • tpm: retrieve digest size of unknown algorithms from TPM
Related show

Commit Message

Roberto Sassu Nov. 14, 2018, 3:31 p.m. UTC
Currently, tpm_pcr_extend() accepts as an input only a SHA1 digest.

This patch modifies the definition of tpm_pcr_extend() to allow other
kernel subsystems to pass a digest for each algorithm supported by the TPM.
All digests are processed by the TPM in one operation.

If a tpm_pcr_extend() caller provides a subset of the supported algorithms,
the TPM driver extends the remaining PCR banks with the first digest
passed as an argument to the function.

The new tpm_bank_list structure has been preferred to the tpm_digest
structure, to let the caller specify the size of the digest (which may be
unknown to the TPM driver).

Due to the API change, ima_pcr_extend() and pcrlock() have been modified.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 drivers/char/tpm/tpm-interface.c   | 24 +++++---------------
 drivers/char/tpm/tpm.h             |  6 ++---
 drivers/char/tpm/tpm1-cmd.c        | 14 ++++++++----
 drivers/char/tpm/tpm2-cmd.c        | 36 +++++++++++++++++++++---------
 include/linux/tpm.h                | 13 ++++++++---
 security/integrity/ima/ima_queue.c |  5 ++++-
 security/keys/trusted.c            |  5 ++++-
 7 files changed, 63 insertions(+), 40 deletions(-)

Comments

Jarkko Sakkinen Nov. 16, 2018, 3:03 p.m. UTC | #1
On Wed, Nov 14, 2018 at 04:31:08PM +0100, Roberto Sassu wrote:
> Currently, tpm_pcr_extend() accepts as an input only a SHA1 digest.
> 
> This patch modifies the definition of tpm_pcr_extend() to allow other
> kernel subsystems to pass a digest for each algorithm supported by the TPM.
> All digests are processed by the TPM in one operation.
> 
> If a tpm_pcr_extend() caller provides a subset of the supported algorithms,
> the TPM driver extends the remaining PCR banks with the first digest
> passed as an argument to the function.

What is the legit use case for this?

> The new tpm_bank_list structure has been preferred to the tpm_digest
> structure, to let the caller specify the size of the digest (which may be
> unknown to the TPM driver).
> 
> Due to the API change, ima_pcr_extend() and pcrlock() have been modified.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Should be dropped from the patch set as it is not taken advatange of
but I can still try to give some feedback.

> ---
>  drivers/char/tpm/tpm-interface.c   | 24 +++++---------------
>  drivers/char/tpm/tpm.h             |  6 ++---
>  drivers/char/tpm/tpm1-cmd.c        | 14 ++++++++----
>  drivers/char/tpm/tpm2-cmd.c        | 36 +++++++++++++++++++++---------
>  include/linux/tpm.h                | 13 ++++++++---
>  security/integrity/ima/ima_queue.c |  5 ++++-
>  security/keys/trusted.c            |  5 ++++-
>  7 files changed, 63 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index f0e1456440d7..5495223d3f7b 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -478,42 +478,30 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
>   * tpm_pcr_extend - extend a PCR value in SHA1 bank.
>   * @chip:	a &struct tpm_chip instance, %NULL for the default chip
>   * @pcr_idx:	the PCR to be retrieved
> - * @hash:	the hash value used to extend the PCR value
> + * @count:	number of tpm_bank_list structures
> + * @bank_list:	array of tpm_bank_list structures used to extend the PCR value
>   *
>   * Note: with TPM 2.0 extends also those banks for which no digest was
>   * specified in order to prevent malicious use of those PCR banks.
>   *
>   * Return: same as with tpm_transmit_cmd()
>   */
> -int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
> +int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> +		   const struct tpm_bank_list *bank_list)
>  {
>  	int rc;
> -	struct tpm_digest *digest_list;
> -	int i;
>  
>  	chip = tpm_find_get_ops(chip);
>  	if (!chip)
>  		return -ENODEV;
>  
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> -		digest_list = kcalloc(chip->nr_allocated_banks,
> -				      sizeof(*digest_list), GFP_KERNEL);
> -		if (!digest_list)
> -			return -ENOMEM;
> -
> -		for (i = 0; i < chip->nr_allocated_banks; i++) {
> -			digest_list[i].alg_id = chip->allocated_banks[i].alg_id;
> -			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
> -		}
> -
> -		rc = tpm2_pcr_extend(chip, pcr_idx, chip->nr_allocated_banks,
> -				     digest_list);
> -		kfree(digest_list);
> +		rc = tpm2_pcr_extend(chip, pcr_idx, count, bank_list);
>  		tpm_put_ops(chip);
>  		return rc;
>  	}
>  
> -	rc = tpm1_pcr_extend(chip, pcr_idx, hash,
> +	rc = tpm1_pcr_extend(chip, pcr_idx, count, bank_list,
>  			     "attempting extend a PCR value");
>  	tpm_put_ops(chip);
>  	return rc;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 023ddf42038b..4296c720ebb4 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -504,8 +504,8 @@ int tpm1_auto_startup(struct tpm_chip *chip);
>  int tpm1_do_selftest(struct tpm_chip *chip);
>  int tpm1_get_timeouts(struct tpm_chip *chip);
>  unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> -int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
> -		    const char *log_msg);
> +int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> +		    const struct tpm_bank_list *bank_list, const char *log_msg);
>  int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
>  ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>  		    const char *desc, size_t min_cap_length);
> @@ -551,7 +551,7 @@ int tpm2_get_timeouts(struct tpm_chip *chip);
>  int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
>  		  struct tpm_digest *digest_struct, u16 *digest_size_ptr);
>  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> -		    struct tpm_digest *digests);
> +		    const struct tpm_bank_list *bank_list);
>  int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
>  void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>  			    unsigned int flags);
> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> index 8b70a7f884a7..439ac749517c 100644
> --- a/drivers/char/tpm/tpm1-cmd.c
> +++ b/drivers/char/tpm/tpm1-cmd.c
> @@ -449,12 +449,19 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
>  }
>  
>  #define TPM_ORD_PCR_EXTEND 20
> -int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
> -		    const char *log_msg)
> +int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> +		    const struct tpm_bank_list *bank_list, const char *log_msg)
>  {
>  	struct tpm_buf buf;
> +	u8 dummy_hash[TPM_DIGEST_SIZE] = { 0 };
> +	const u8 *hash;
>  	int rc;
>  
> +	hash = dummy_hash;
> +	if (count)
> +		memcpy(dummy_hash, bank_list[0].extend_array,
> +		       min(bank_list[0].digest_size, (u16)sizeof(dummy_hash)));
> +
>  	rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);

This is probably mistake? I mean dummy_hash is not used.

>  	if (rc)
>  		return rc;
> @@ -743,7 +750,6 @@ int tpm1_auto_startup(struct tpm_chip *chip)
>   */
>  int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
>  {
> -	u8 dummy_hash[TPM_DIGEST_SIZE] = { 0 };
>  	struct tpm_buf buf;
>  	unsigned int try;
>  	int rc;
> @@ -751,7 +757,7 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
>  
>  	/* for buggy tpm, flush pcrs with extend to selected dummy */
>  	if (tpm_suspend_pcr)
> -		rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, dummy_hash,
> +		rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, 0, NULL,
>  				     "extending dummy pcr before suspend");
>  
>  	rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 974465f04b78..40eb1a044451 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -248,21 +248,22 @@ struct tpm2_null_auth_area {
>   *
>   * @chip:	TPM chip to use.
>   * @pcr_idx:	index of the PCR.
> - * @count:	number of digests passed.
> - * @digests:	list of pcr banks and corresponding digest values to extend.
> + * @count:	number of tpm_bank_list passed.
> + * @bank_list:	array of tpm_bank_list with digest values to extend.
>   *
>   * Return: Same as with tpm_transmit_cmd.
>   */
>  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> -		    struct tpm_digest *digests)
> +		    const struct tpm_bank_list *bank_list)
>  {
>  	struct tpm_buf buf;
>  	struct tpm2_null_auth_area auth_area;
> +	const struct tpm_bank_list *item;
> +	u8 dummy_hash[SHA512_DIGEST_SIZE] = { 0 };
> +	const u8 *hash;
>  	int rc;
>  	int i;
> -
> -	if (count > chip->nr_allocated_banks)
> -		return -EINVAL;
> +	int j;
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
>  	if (rc)
> @@ -278,11 +279,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>  	tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
>  	tpm_buf_append(&buf, (const unsigned char *)&auth_area,
>  		       sizeof(auth_area));
> -	tpm_buf_append_u32(&buf, count);
> +	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
> +
> +	if (count)
> +		memcpy(dummy_hash, bank_list[0].extend_array,
> +		       bank_list[0].digest_size);
> +
> +	for (i = 0; i < chip->nr_allocated_banks; i++) {
> +		tpm_buf_append_u16(&buf, chip->allocated_banks[i].alg_id);
> +
> +		hash = dummy_hash;
> +		for (j = 0; j < count; j++) {
> +			item = bank_list + j;
> +
> +			if (item->alg_id == chip->allocated_banks[i].alg_id) {
> +				hash = item->extend_array;
> +				break;
> +			}
> +		}
>  
> -	for (i = 0; i < count; i++) {
> -		tpm_buf_append_u16(&buf, digests[i].alg_id);
> -		tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
> +		tpm_buf_append(&buf, hash,
>  			       chip->allocated_banks[i].digest_size);
>  	}
>  
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index fcdd33ae9969..16e5ff1f0294 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -52,6 +52,12 @@ struct tpm_bank_info {
>  	u16 crypto_id;
>  };
>  
> +struct tpm_bank_list {
> +	u8 alg_id;
> +	u16 digest_size;
> +	const u8 *extend_array;
> +};

You should document this structure.

> +
>  enum TPM_OPS_FLAGS {
>  	TPM_OPS_AUTO_STARTUP = BIT(0),
>  };
> @@ -79,7 +85,8 @@ struct tpm_class_ops {
>  extern int tpm_is_tpm2(struct tpm_chip *chip);
>  extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
>  			struct tpm_digest *digest_struct);
> -extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash);
> +extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> +			  const struct tpm_bank_list *bank_list);
>  extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
>  extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
>  extern int tpm_seal_trusted(struct tpm_chip *chip,
> @@ -101,8 +108,8 @@ static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
>  	return -ENODEV;
>  }
>  
> -static inline int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> -				 const u8 *hash)
> +static inline int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> +				 const struct tpm_bank_list *bank_list)
>  {
>  	return -ENODEV;
>  }
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index b186819bd5aa..24024edef316 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -140,12 +140,15 @@ unsigned long ima_get_binary_runtime_size(void)
>  
>  static int ima_pcr_extend(const u8 *hash, int pcr)
>  {
> +	struct tpm_bank_list bank_item = { .alg_id = TPM_ALG_SHA1,
> +					   .digest_size = TPM_DIGEST_SIZE,
> +					   .extend_array = hash };

Item is a list?

>  	int result = 0;
>  
>  	if (!ima_tpm_chip)
>  		return result;
>  
> -	result = tpm_pcr_extend(ima_tpm_chip, pcr, hash);
> +	result = tpm_pcr_extend(ima_tpm_chip, pcr, 1, &bank_item);
>  	if (result != 0)
>  		pr_err("Error Communicating to TPM chip, result: %d\n", result);
>  	return result;
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index ff6789365a12..d2a3129a95f9 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -380,6 +380,9 @@ EXPORT_SYMBOL_GPL(trusted_tpm_send);
>  static int pcrlock(const int pcrnum)
>  {
>  	unsigned char hash[SHA1_DIGEST_SIZE];
> +	struct tpm_bank_list bank_item = { .alg_id = TPM_ALG_SHA1,
> +					   .digest_size = sizeof(hash),
> +					   .extend_array = hash }
>  	int ret;
>  
>  	if (!capable(CAP_SYS_ADMIN))
> @@ -387,7 +390,7 @@ static int pcrlock(const int pcrnum)
>  	ret = tpm_get_random(NULL, hash, SHA1_DIGEST_SIZE);
>  	if (ret != SHA1_DIGEST_SIZE)
>  		return ret;
> -	return tpm_pcr_extend(NULL, pcrnum, hash) ? -EINVAL : 0;
> +	return tpm_pcr_extend(NULL, pcrnum, 1, &bank_item) ? -EINVAL : 0;
>  }
>  
>  /*
> -- 
> 2.17.1
> 

/Jarkko
Roberto Sassu Nov. 16, 2018, 3:55 p.m. UTC | #2
On 11/16/2018 4:03 PM, Jarkko Sakkinen wrote:
> On Wed, Nov 14, 2018 at 04:31:08PM +0100, Roberto Sassu wrote:
>> Currently, tpm_pcr_extend() accepts as an input only a SHA1 digest.
>>
>> This patch modifies the definition of tpm_pcr_extend() to allow other
>> kernel subsystems to pass a digest for each algorithm supported by the TPM.
>> All digests are processed by the TPM in one operation.
>>
>> If a tpm_pcr_extend() caller provides a subset of the supported algorithms,
>> the TPM driver extends the remaining PCR banks with the first digest
>> passed as an argument to the function.
> 
> What is the legit use case for this?

A subset could be chosen for better performance, or when a TPM algorithm
is not supported by the crypto subsystem.


>> The new tpm_bank_list structure has been preferred to the tpm_digest
>> structure, to let the caller specify the size of the digest (which may be
>> unknown to the TPM driver).
>>
>> Due to the API change, ima_pcr_extend() and pcrlock() have been modified.
>>
>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Should be dropped from the patch set as it is not taken advatange of
> but I can still try to give some feedback.

I understood from a previous email that you want to include all API
changes for crypto agility in the same patch set.

Roberto


>> ---
>>   drivers/char/tpm/tpm-interface.c   | 24 +++++---------------
>>   drivers/char/tpm/tpm.h             |  6 ++---
>>   drivers/char/tpm/tpm1-cmd.c        | 14 ++++++++----
>>   drivers/char/tpm/tpm2-cmd.c        | 36 +++++++++++++++++++++---------
>>   include/linux/tpm.h                | 13 ++++++++---
>>   security/integrity/ima/ima_queue.c |  5 ++++-
>>   security/keys/trusted.c            |  5 ++++-
>>   7 files changed, 63 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> index f0e1456440d7..5495223d3f7b 100644
>> --- a/drivers/char/tpm/tpm-interface.c
>> +++ b/drivers/char/tpm/tpm-interface.c
>> @@ -478,42 +478,30 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
>>    * tpm_pcr_extend - extend a PCR value in SHA1 bank.
>>    * @chip:	a &struct tpm_chip instance, %NULL for the default chip
>>    * @pcr_idx:	the PCR to be retrieved
>> - * @hash:	the hash value used to extend the PCR value
>> + * @count:	number of tpm_bank_list structures
>> + * @bank_list:	array of tpm_bank_list structures used to extend the PCR value
>>    *
>>    * Note: with TPM 2.0 extends also those banks for which no digest was
>>    * specified in order to prevent malicious use of those PCR banks.
>>    *
>>    * Return: same as with tpm_transmit_cmd()
>>    */
>> -int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
>> +int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>> +		   const struct tpm_bank_list *bank_list)
>>   {
>>   	int rc;
>> -	struct tpm_digest *digest_list;
>> -	int i;
>>   
>>   	chip = tpm_find_get_ops(chip);
>>   	if (!chip)
>>   		return -ENODEV;
>>   
>>   	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>> -		digest_list = kcalloc(chip->nr_allocated_banks,
>> -				      sizeof(*digest_list), GFP_KERNEL);
>> -		if (!digest_list)
>> -			return -ENOMEM;
>> -
>> -		for (i = 0; i < chip->nr_allocated_banks; i++) {
>> -			digest_list[i].alg_id = chip->allocated_banks[i].alg_id;
>> -			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
>> -		}
>> -
>> -		rc = tpm2_pcr_extend(chip, pcr_idx, chip->nr_allocated_banks,
>> -				     digest_list);
>> -		kfree(digest_list);
>> +		rc = tpm2_pcr_extend(chip, pcr_idx, count, bank_list);
>>   		tpm_put_ops(chip);
>>   		return rc;
>>   	}
>>   
>> -	rc = tpm1_pcr_extend(chip, pcr_idx, hash,
>> +	rc = tpm1_pcr_extend(chip, pcr_idx, count, bank_list,
>>   			     "attempting extend a PCR value");
>>   	tpm_put_ops(chip);
>>   	return rc;
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 023ddf42038b..4296c720ebb4 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -504,8 +504,8 @@ int tpm1_auto_startup(struct tpm_chip *chip);
>>   int tpm1_do_selftest(struct tpm_chip *chip);
>>   int tpm1_get_timeouts(struct tpm_chip *chip);
>>   unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>> -int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
>> -		    const char *log_msg);
>> +int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>> +		    const struct tpm_bank_list *bank_list, const char *log_msg);
>>   int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
>>   ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>>   		    const char *desc, size_t min_cap_length);
>> @@ -551,7 +551,7 @@ int tpm2_get_timeouts(struct tpm_chip *chip);
>>   int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
>>   		  struct tpm_digest *digest_struct, u16 *digest_size_ptr);
>>   int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>> -		    struct tpm_digest *digests);
>> +		    const struct tpm_bank_list *bank_list);
>>   int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
>>   void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>>   			    unsigned int flags);
>> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
>> index 8b70a7f884a7..439ac749517c 100644
>> --- a/drivers/char/tpm/tpm1-cmd.c
>> +++ b/drivers/char/tpm/tpm1-cmd.c
>> @@ -449,12 +449,19 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
>>   }
>>   
>>   #define TPM_ORD_PCR_EXTEND 20
>> -int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
>> -		    const char *log_msg)
>> +int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>> +		    const struct tpm_bank_list *bank_list, const char *log_msg)
>>   {
>>   	struct tpm_buf buf;
>> +	u8 dummy_hash[TPM_DIGEST_SIZE] = { 0 };
>> +	const u8 *hash;
>>   	int rc;
>>   
>> +	hash = dummy_hash;
>> +	if (count)
>> +		memcpy(dummy_hash, bank_list[0].extend_array,
>> +		       min(bank_list[0].digest_size, (u16)sizeof(dummy_hash)));
>> +
>>   	rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
> 
> This is probably mistake? I mean dummy_hash is not used.
> 
>>   	if (rc)
>>   		return rc;
>> @@ -743,7 +750,6 @@ int tpm1_auto_startup(struct tpm_chip *chip)
>>    */
>>   int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
>>   {
>> -	u8 dummy_hash[TPM_DIGEST_SIZE] = { 0 };
>>   	struct tpm_buf buf;
>>   	unsigned int try;
>>   	int rc;
>> @@ -751,7 +757,7 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
>>   
>>   	/* for buggy tpm, flush pcrs with extend to selected dummy */
>>   	if (tpm_suspend_pcr)
>> -		rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, dummy_hash,
>> +		rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, 0, NULL,
>>   				     "extending dummy pcr before suspend");
>>   
>>   	rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 974465f04b78..40eb1a044451 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -248,21 +248,22 @@ struct tpm2_null_auth_area {
>>    *
>>    * @chip:	TPM chip to use.
>>    * @pcr_idx:	index of the PCR.
>> - * @count:	number of digests passed.
>> - * @digests:	list of pcr banks and corresponding digest values to extend.
>> + * @count:	number of tpm_bank_list passed.
>> + * @bank_list:	array of tpm_bank_list with digest values to extend.
>>    *
>>    * Return: Same as with tpm_transmit_cmd.
>>    */
>>   int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>> -		    struct tpm_digest *digests)
>> +		    const struct tpm_bank_list *bank_list)
>>   {
>>   	struct tpm_buf buf;
>>   	struct tpm2_null_auth_area auth_area;
>> +	const struct tpm_bank_list *item;
>> +	u8 dummy_hash[SHA512_DIGEST_SIZE] = { 0 };
>> +	const u8 *hash;
>>   	int rc;
>>   	int i;
>> -
>> -	if (count > chip->nr_allocated_banks)
>> -		return -EINVAL;
>> +	int j;
>>   
>>   	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
>>   	if (rc)
>> @@ -278,11 +279,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>>   	tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
>>   	tpm_buf_append(&buf, (const unsigned char *)&auth_area,
>>   		       sizeof(auth_area));
>> -	tpm_buf_append_u32(&buf, count);
>> +	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
>> +
>> +	if (count)
>> +		memcpy(dummy_hash, bank_list[0].extend_array,
>> +		       bank_list[0].digest_size);
>> +
>> +	for (i = 0; i < chip->nr_allocated_banks; i++) {
>> +		tpm_buf_append_u16(&buf, chip->allocated_banks[i].alg_id);
>> +
>> +		hash = dummy_hash;
>> +		for (j = 0; j < count; j++) {
>> +			item = bank_list + j;
>> +
>> +			if (item->alg_id == chip->allocated_banks[i].alg_id) {
>> +				hash = item->extend_array;
>> +				break;
>> +			}
>> +		}
>>   
>> -	for (i = 0; i < count; i++) {
>> -		tpm_buf_append_u16(&buf, digests[i].alg_id);
>> -		tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
>> +		tpm_buf_append(&buf, hash,
>>   			       chip->allocated_banks[i].digest_size);
>>   	}
>>   
>> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>> index fcdd33ae9969..16e5ff1f0294 100644
>> --- a/include/linux/tpm.h
>> +++ b/include/linux/tpm.h
>> @@ -52,6 +52,12 @@ struct tpm_bank_info {
>>   	u16 crypto_id;
>>   };
>>   
>> +struct tpm_bank_list {
>> +	u8 alg_id;
>> +	u16 digest_size;
>> +	const u8 *extend_array;
>> +};
> 
> You should document this structure.
> 
>> +
>>   enum TPM_OPS_FLAGS {
>>   	TPM_OPS_AUTO_STARTUP = BIT(0),
>>   };
>> @@ -79,7 +85,8 @@ struct tpm_class_ops {
>>   extern int tpm_is_tpm2(struct tpm_chip *chip);
>>   extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
>>   			struct tpm_digest *digest_struct);
>> -extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash);
>> +extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>> +			  const struct tpm_bank_list *bank_list);
>>   extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
>>   extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
>>   extern int tpm_seal_trusted(struct tpm_chip *chip,
>> @@ -101,8 +108,8 @@ static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
>>   	return -ENODEV;
>>   }
>>   
>> -static inline int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
>> -				 const u8 *hash)
>> +static inline int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>> +				 const struct tpm_bank_list *bank_list)
>>   {
>>   	return -ENODEV;
>>   }
>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>> index b186819bd5aa..24024edef316 100644
>> --- a/security/integrity/ima/ima_queue.c
>> +++ b/security/integrity/ima/ima_queue.c
>> @@ -140,12 +140,15 @@ unsigned long ima_get_binary_runtime_size(void)
>>   
>>   static int ima_pcr_extend(const u8 *hash, int pcr)
>>   {
>> +	struct tpm_bank_list bank_item = { .alg_id = TPM_ALG_SHA1,
>> +					   .digest_size = TPM_DIGEST_SIZE,
>> +					   .extend_array = hash };
> 
> Item is a list?
> 
>>   	int result = 0;
>>   
>>   	if (!ima_tpm_chip)
>>   		return result;
>>   
>> -	result = tpm_pcr_extend(ima_tpm_chip, pcr, hash);
>> +	result = tpm_pcr_extend(ima_tpm_chip, pcr, 1, &bank_item);
>>   	if (result != 0)
>>   		pr_err("Error Communicating to TPM chip, result: %d\n", result);
>>   	return result;
>> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
>> index ff6789365a12..d2a3129a95f9 100644
>> --- a/security/keys/trusted.c
>> +++ b/security/keys/trusted.c
>> @@ -380,6 +380,9 @@ EXPORT_SYMBOL_GPL(trusted_tpm_send);
>>   static int pcrlock(const int pcrnum)
>>   {
>>   	unsigned char hash[SHA1_DIGEST_SIZE];
>> +	struct tpm_bank_list bank_item = { .alg_id = TPM_ALG_SHA1,
>> +					   .digest_size = sizeof(hash),
>> +					   .extend_array = hash }
>>   	int ret;
>>   
>>   	if (!capable(CAP_SYS_ADMIN))
>> @@ -387,7 +390,7 @@ static int pcrlock(const int pcrnum)
>>   	ret = tpm_get_random(NULL, hash, SHA1_DIGEST_SIZE);
>>   	if (ret != SHA1_DIGEST_SIZE)
>>   		return ret;
>> -	return tpm_pcr_extend(NULL, pcrnum, hash) ? -EINVAL : 0;
>> +	return tpm_pcr_extend(NULL, pcrnum, 1, &bank_item) ? -EINVAL : 0;
>>   }
>>   
>>   /*
>> -- 
>> 2.17.1
>>
> 
> /Jarkko
>
Jarkko Sakkinen Nov. 18, 2018, 7:27 a.m. UTC | #3
On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
> On 11/16/2018 4:03 PM, Jarkko Sakkinen wrote:
> > On Wed, Nov 14, 2018 at 04:31:08PM +0100, Roberto Sassu wrote:
> > > Currently, tpm_pcr_extend() accepts as an input only a SHA1 digest.
> > > 
> > > This patch modifies the definition of tpm_pcr_extend() to allow other
> > > kernel subsystems to pass a digest for each algorithm supported by the TPM.
> > > All digests are processed by the TPM in one operation.
> > > 
> > > If a tpm_pcr_extend() caller provides a subset of the supported algorithms,
> > > the TPM driver extends the remaining PCR banks with the first digest
> > > passed as an argument to the function.
> > 
> > What is the legit use case for this?
> 
> A subset could be chosen for better performance, or when a TPM algorithm
> is not supported by the crypto subsystem.

Doesn't extending a subset a security concern?

/Jarkko
Jarkko Sakkinen Nov. 18, 2018, 7:29 a.m. UTC | #4
On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
> I understood from a previous email that you want to include all API
> changes for crypto agility in the same patch set.

Hmm.. maybe there is some misunderstading. Can you point me to the
comment? Thanks.

/Jarkko
Mimi Zohar Nov. 19, 2018, 4:57 a.m. UTC | #5
On Sun, 2018-11-18 at 09:27 +0200, Jarkko Sakkinen wrote:
> On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
> > On 11/16/2018 4:03 PM, Jarkko Sakkinen wrote:
> > > On Wed, Nov 14, 2018 at 04:31:08PM +0100, Roberto Sassu wrote:
> > > > Currently, tpm_pcr_extend() accepts as an input only a SHA1 digest.
> > > > 
> > > > This patch modifies the definition of tpm_pcr_extend() to allow other
> > > > kernel subsystems to pass a digest for each algorithm supported by the TPM.
> > > > All digests are processed by the TPM in one operation.
> > > > 
> > > > If a tpm_pcr_extend() caller provides a subset of the supported algorithms,
> > > > the TPM driver extends the remaining PCR banks with the first digest
> > > > passed as an argument to the function.
> > > 
> > > What is the legit use case for this?
> > 
> > A subset could be chosen for better performance, or when a TPM algorithm
> > is not supported by the crypto subsystem.
> 
> Doesn't extending a subset a security concern?

Right, so instead of extending a subset of the allocated banks, all of
the allocated banks need to be extended, even for those banks that a
digest was not included.  This is no different than what is being done
today.  IMA is currently only calculating the SHA1 hash, padding the
digest with 0's, and extending the padded value(s) into all of the
allocated banks.

If there is a vulnerability with the hash algorithm, then any bank
extended with the padded/truncated digest would be susceptible.

IMA will need to become TPM 2.0 aware, calculating and extending
multiple banks and define a new measurement list format containing the
multiple digests.

Mimi
Roberto Sassu Nov. 19, 2018, 8:16 a.m. UTC | #6
On 11/19/2018 5:57 AM, Mimi Zohar wrote:
> On Sun, 2018-11-18 at 09:27 +0200, Jarkko Sakkinen wrote:
>> On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
>>> On 11/16/2018 4:03 PM, Jarkko Sakkinen wrote:
>>>> On Wed, Nov 14, 2018 at 04:31:08PM +0100, Roberto Sassu wrote:
>>>>> Currently, tpm_pcr_extend() accepts as an input only a SHA1 digest.
>>>>>
>>>>> This patch modifies the definition of tpm_pcr_extend() to allow other
>>>>> kernel subsystems to pass a digest for each algorithm supported by the TPM.
>>>>> All digests are processed by the TPM in one operation.
>>>>>
>>>>> If a tpm_pcr_extend() caller provides a subset of the supported algorithms,
>>>>> the TPM driver extends the remaining PCR banks with the first digest
>>>>> passed as an argument to the function.
>>>>
>>>> What is the legit use case for this?
>>>
>>> A subset could be chosen for better performance, or when a TPM algorithm
>>> is not supported by the crypto subsystem.
>>
>> Doesn't extending a subset a security concern?
> 
> Right, so instead of extending a subset of the allocated banks, all of
> the allocated banks need to be extended, even for those banks that a
> digest was not included.  This is no different than what is being done
> today.  IMA is currently only calculating the SHA1 hash, padding the
> digest with 0's, and extending the padded value(s) into all of the
> allocated banks.

The caller of tpm_pcr_extend() could pass a subset of the allocated
banks, but the TPM driver extends all banks as before.

Roberto


> If there is a vulnerability with the hash algorithm, then any bank
> extended with the padded/truncated digest would be susceptible.
> 
> IMA will need to become TPM 2.0 aware, calculating and extending
> multiple banks and define a new measurement list format containing the
> multiple digests.
> 
> Mimi
>
Roberto Sassu Nov. 19, 2018, 8:22 a.m. UTC | #7
On 11/18/2018 8:29 AM, Jarkko Sakkinen wrote:
> On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
>> I understood from a previous email that you want to include all API
>> changes for crypto agility in the same patch set.
> 
> Hmm.. maybe there is some misunderstading. Can you point me to the
> comment? Thanks.

---
On Thu, Nov 08, 2018 at 03:24:46PM +0100, Roberto Sassu wrote:
 > Should I include the patch for tpm_pcr_extend() in this patch set, even
 > if the set fixes the digest size issue?

Just add some note to the cover letter. Makes sense here to have a
prequel patch for that because otherwise the implementation will be
a bit messy and half-baked.
---

Roberto


> /Jarkko
>
Mimi Zohar Nov. 19, 2018, 2:33 p.m. UTC | #8
On Mon, 2018-11-19 at 09:16 +0100, Roberto Sassu wrote:
> On 11/19/2018 5:57 AM, Mimi Zohar wrote:
> > On Sun, 2018-11-18 at 09:27 +0200, Jarkko Sakkinen wrote:
> >> On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
> >>> On 11/16/2018 4:03 PM, Jarkko Sakkinen wrote:
> >>>> On Wed, Nov 14, 2018 at 04:31:08PM +0100, Roberto Sassu wrote:
> >>>>> Currently, tpm_pcr_extend() accepts as an input only a SHA1 digest.
> >>>>>
> >>>>> This patch modifies the definition of tpm_pcr_extend() to allow other
> >>>>> kernel subsystems to pass a digest for each algorithm supported by the TPM.
> >>>>> All digests are processed by the TPM in one operation.
> >>>>>
> >>>>> If a tpm_pcr_extend() caller provides a subset of the supported algorithms,
> >>>>> the TPM driver extends the remaining PCR banks with the first digest
> >>>>> passed as an argument to the function.
> >>>>
> >>>> What is the legit use case for this?
> >>>
> >>> A subset could be chosen for better performance, or when a TPM algorithm
> >>> is not supported by the crypto subsystem.
> >>
> >> Doesn't extending a subset a security concern?
> > 
> > Right, so instead of extending a subset of the allocated banks, all of
> > the allocated banks need to be extended, even for those banks that a
> > digest was not included.  This is no different than what is being done
> > today.  IMA is currently only calculating the SHA1 hash, padding the
> > digest with 0's, and extending the padded value(s) into all of the
> > allocated banks.
> 
> The caller of tpm_pcr_extend() could pass a subset of the allocated
> banks, but the TPM driver extends all banks as before.

Agreed, there should be a clear division.

1) The caller shouldn't need to know anything about the chip->info.
2) The TPM driver should not rely on the caller to supply all the
hashes, but verify that all allocated banks are being extended.

Mimi

> 
> 
> > If there is a vulnerability with the hash algorithm, then any bank
> > extended with the padded/truncated digest would be susceptible.
> > 
> > IMA will need to become TPM 2.0 aware, calculating and extending
> > multiple banks and define a new measurement list format containing the
> > multiple digests.
> > 
> > Mimi
> > 
>
Jarkko Sakkinen Nov. 19, 2018, 2:39 p.m. UTC | #9
On Mon, Nov 19, 2018 at 09:16:45AM +0100, Roberto Sassu wrote:
> The caller of tpm_pcr_extend() could pass a subset of the allocated
> banks, but the TPM driver extends all banks as before.

Ah, of course. Then it should be legit I guess...

/Jarkko
Jarkko Sakkinen Nov. 19, 2018, 2:42 p.m. UTC | #10
On Mon, Nov 19, 2018 at 09:22:32AM +0100, Roberto Sassu wrote:
> On 11/18/2018 8:29 AM, Jarkko Sakkinen wrote:
> > On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
> > > I understood from a previous email that you want to include all API
> > > changes for crypto agility in the same patch set.
> > 
> > Hmm.. maybe there is some misunderstading. Can you point me to the
> > comment? Thanks.
> 
> ---
> On Thu, Nov 08, 2018 at 03:24:46PM +0100, Roberto Sassu wrote:
> > Should I include the patch for tpm_pcr_extend() in this patch set, even
> > if the set fixes the digest size issue?
> 
> Just add some note to the cover letter. Makes sense here to have a
> prequel patch for that because otherwise the implementation will be
> a bit messy and half-baked.

Ok, I guess it is OK as long as IMA (namely Mimi) gives that patch
reviewed-by. It is not a huge change.

PS. I noticed that earlier patches have postfixes like '_struct' and
'_ptr' for function arguments. Can you remove them in the next version?

/Jarkko

Patch
diff mbox series

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index f0e1456440d7..5495223d3f7b 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -478,42 +478,30 @@  EXPORT_SYMBOL_GPL(tpm_pcr_read);
  * tpm_pcr_extend - extend a PCR value in SHA1 bank.
  * @chip:	a &struct tpm_chip instance, %NULL for the default chip
  * @pcr_idx:	the PCR to be retrieved
- * @hash:	the hash value used to extend the PCR value
+ * @count:	number of tpm_bank_list structures
+ * @bank_list:	array of tpm_bank_list structures used to extend the PCR value
  *
  * Note: with TPM 2.0 extends also those banks for which no digest was
  * specified in order to prevent malicious use of those PCR banks.
  *
  * Return: same as with tpm_transmit_cmd()
  */
-int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
+int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
+		   const struct tpm_bank_list *bank_list)
 {
 	int rc;
-	struct tpm_digest *digest_list;
-	int i;
 
 	chip = tpm_find_get_ops(chip);
 	if (!chip)
 		return -ENODEV;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		digest_list = kcalloc(chip->nr_allocated_banks,
-				      sizeof(*digest_list), GFP_KERNEL);
-		if (!digest_list)
-			return -ENOMEM;
-
-		for (i = 0; i < chip->nr_allocated_banks; i++) {
-			digest_list[i].alg_id = chip->allocated_banks[i].alg_id;
-			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
-		}
-
-		rc = tpm2_pcr_extend(chip, pcr_idx, chip->nr_allocated_banks,
-				     digest_list);
-		kfree(digest_list);
+		rc = tpm2_pcr_extend(chip, pcr_idx, count, bank_list);
 		tpm_put_ops(chip);
 		return rc;
 	}
 
-	rc = tpm1_pcr_extend(chip, pcr_idx, hash,
+	rc = tpm1_pcr_extend(chip, pcr_idx, count, bank_list,
 			     "attempting extend a PCR value");
 	tpm_put_ops(chip);
 	return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 023ddf42038b..4296c720ebb4 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -504,8 +504,8 @@  int tpm1_auto_startup(struct tpm_chip *chip);
 int tpm1_do_selftest(struct tpm_chip *chip);
 int tpm1_get_timeouts(struct tpm_chip *chip);
 unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
-int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
-		    const char *log_msg);
+int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
+		    const struct tpm_bank_list *bank_list, const char *log_msg);
 int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
 ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 		    const char *desc, size_t min_cap_length);
@@ -551,7 +551,7 @@  int tpm2_get_timeouts(struct tpm_chip *chip);
 int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 		  struct tpm_digest *digest_struct, u16 *digest_size_ptr);
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
-		    struct tpm_digest *digests);
+		    const struct tpm_bank_list *bank_list);
 int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
 void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 			    unsigned int flags);
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 8b70a7f884a7..439ac749517c 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -449,12 +449,19 @@  int tpm1_get_timeouts(struct tpm_chip *chip)
 }
 
 #define TPM_ORD_PCR_EXTEND 20
-int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
-		    const char *log_msg)
+int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
+		    const struct tpm_bank_list *bank_list, const char *log_msg)
 {
 	struct tpm_buf buf;
+	u8 dummy_hash[TPM_DIGEST_SIZE] = { 0 };
+	const u8 *hash;
 	int rc;
 
+	hash = dummy_hash;
+	if (count)
+		memcpy(dummy_hash, bank_list[0].extend_array,
+		       min(bank_list[0].digest_size, (u16)sizeof(dummy_hash)));
+
 	rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
 	if (rc)
 		return rc;
@@ -743,7 +750,6 @@  int tpm1_auto_startup(struct tpm_chip *chip)
  */
 int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
 {
-	u8 dummy_hash[TPM_DIGEST_SIZE] = { 0 };
 	struct tpm_buf buf;
 	unsigned int try;
 	int rc;
@@ -751,7 +757,7 @@  int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
 
 	/* for buggy tpm, flush pcrs with extend to selected dummy */
 	if (tpm_suspend_pcr)
-		rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, dummy_hash,
+		rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, 0, NULL,
 				     "extending dummy pcr before suspend");
 
 	rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 974465f04b78..40eb1a044451 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -248,21 +248,22 @@  struct tpm2_null_auth_area {
  *
  * @chip:	TPM chip to use.
  * @pcr_idx:	index of the PCR.
- * @count:	number of digests passed.
- * @digests:	list of pcr banks and corresponding digest values to extend.
+ * @count:	number of tpm_bank_list passed.
+ * @bank_list:	array of tpm_bank_list with digest values to extend.
  *
  * Return: Same as with tpm_transmit_cmd.
  */
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
-		    struct tpm_digest *digests)
+		    const struct tpm_bank_list *bank_list)
 {
 	struct tpm_buf buf;
 	struct tpm2_null_auth_area auth_area;
+	const struct tpm_bank_list *item;
+	u8 dummy_hash[SHA512_DIGEST_SIZE] = { 0 };
+	const u8 *hash;
 	int rc;
 	int i;
-
-	if (count > chip->nr_allocated_banks)
-		return -EINVAL;
+	int j;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
 	if (rc)
@@ -278,11 +279,26 @@  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
 	tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
 	tpm_buf_append(&buf, (const unsigned char *)&auth_area,
 		       sizeof(auth_area));
-	tpm_buf_append_u32(&buf, count);
+	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
+
+	if (count)
+		memcpy(dummy_hash, bank_list[0].extend_array,
+		       bank_list[0].digest_size);
+
+	for (i = 0; i < chip->nr_allocated_banks; i++) {
+		tpm_buf_append_u16(&buf, chip->allocated_banks[i].alg_id);
+
+		hash = dummy_hash;
+		for (j = 0; j < count; j++) {
+			item = bank_list + j;
+
+			if (item->alg_id == chip->allocated_banks[i].alg_id) {
+				hash = item->extend_array;
+				break;
+			}
+		}
 
-	for (i = 0; i < count; i++) {
-		tpm_buf_append_u16(&buf, digests[i].alg_id);
-		tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
+		tpm_buf_append(&buf, hash,
 			       chip->allocated_banks[i].digest_size);
 	}
 
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index fcdd33ae9969..16e5ff1f0294 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -52,6 +52,12 @@  struct tpm_bank_info {
 	u16 crypto_id;
 };
 
+struct tpm_bank_list {
+	u8 alg_id;
+	u16 digest_size;
+	const u8 *extend_array;
+};
+
 enum TPM_OPS_FLAGS {
 	TPM_OPS_AUTO_STARTUP = BIT(0),
 };
@@ -79,7 +85,8 @@  struct tpm_class_ops {
 extern int tpm_is_tpm2(struct tpm_chip *chip);
 extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 			struct tpm_digest *digest_struct);
-extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash);
+extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
+			  const struct tpm_bank_list *bank_list);
 extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
 extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
 extern int tpm_seal_trusted(struct tpm_chip *chip,
@@ -101,8 +108,8 @@  static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
 	return -ENODEV;
 }
 
-static inline int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
-				 const u8 *hash)
+static inline int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
+				 const struct tpm_bank_list *bank_list)
 {
 	return -ENODEV;
 }
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index b186819bd5aa..24024edef316 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -140,12 +140,15 @@  unsigned long ima_get_binary_runtime_size(void)
 
 static int ima_pcr_extend(const u8 *hash, int pcr)
 {
+	struct tpm_bank_list bank_item = { .alg_id = TPM_ALG_SHA1,
+					   .digest_size = TPM_DIGEST_SIZE,
+					   .extend_array = hash };
 	int result = 0;
 
 	if (!ima_tpm_chip)
 		return result;
 
-	result = tpm_pcr_extend(ima_tpm_chip, pcr, hash);
+	result = tpm_pcr_extend(ima_tpm_chip, pcr, 1, &bank_item);
 	if (result != 0)
 		pr_err("Error Communicating to TPM chip, result: %d\n", result);
 	return result;
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index ff6789365a12..d2a3129a95f9 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -380,6 +380,9 @@  EXPORT_SYMBOL_GPL(trusted_tpm_send);
 static int pcrlock(const int pcrnum)
 {
 	unsigned char hash[SHA1_DIGEST_SIZE];
+	struct tpm_bank_list bank_item = { .alg_id = TPM_ALG_SHA1,
+					   .digest_size = sizeof(hash),
+					   .extend_array = hash }
 	int ret;
 
 	if (!capable(CAP_SYS_ADMIN))
@@ -387,7 +390,7 @@  static int pcrlock(const int pcrnum)
 	ret = tpm_get_random(NULL, hash, SHA1_DIGEST_SIZE);
 	if (ret != SHA1_DIGEST_SIZE)
 		return ret;
-	return tpm_pcr_extend(NULL, pcrnum, hash) ? -EINVAL : 0;
+	return tpm_pcr_extend(NULL, pcrnum, 1, &bank_item) ? -EINVAL : 0;
 }
 
 /*