diff mbox series

[v7,1/5] tpm: dynamically allocate the allocated_banks array

Message ID 20181213102945.30946-2-roberto.sassu@huawei.com (mailing list archive)
State New, archived
Headers show
Series tpm: retrieve digest size of unknown algorithms from TPM | expand

Commit Message

Roberto Sassu Dec. 13, 2018, 10:29 a.m. UTC
This patch renames active_banks (member of tpm_chip) to allocated_banks,
stores the number of allocated PCR banks in nr_allocated_banks (new member
of tpm_chip), and replaces the static array with a pointer to a dynamically
allocated array.

tpm2_get_pcr_allocation() determines if a PCR bank is allocated by checking
the mask in the TPML_PCR_SELECTION structure returned by the TPM for
TPM2_Get_Capability(). If a bank is not allocated, the TPM returns that
bank in TPML_PCR_SELECTION, with all bits in the mask set to zero. In this
case, the bank is not included in chip->allocated_banks, to avoid that TPM
driver users unnecessarily calculate a digest for that bank.

One PCR bank with algorithm set to SHA1 is always allocated for TPM 1.x.

As a consequence of the introduction of nr_allocated_banks,
tpm_pcr_extend() does not check anymore if the algorithm stored in tpm_chip
is equal to zero.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-chip.c      |  1 +
 drivers/char/tpm/tpm-interface.c | 18 +++++++++--------
 drivers/char/tpm/tpm.h           |  3 ++-
 drivers/char/tpm/tpm1-cmd.c      | 10 ++++++++++
 drivers/char/tpm/tpm2-cmd.c      | 34 ++++++++++++++++++++++----------
 5 files changed, 47 insertions(+), 19 deletions(-)

Comments

Jarkko Sakkinen Dec. 20, 2018, 2:55 p.m. UTC | #1
On Thu, Dec 13, 2018 at 11:29:41AM +0100, Roberto Sassu wrote:
> This patch renames active_banks (member of tpm_chip) to allocated_banks,
> stores the number of allocated PCR banks in nr_allocated_banks (new member
> of tpm_chip), and replaces the static array with a pointer to a dynamically
> allocated array.
> 
> tpm2_get_pcr_allocation() determines if a PCR bank is allocated by checking
> the mask in the TPML_PCR_SELECTION structure returned by the TPM for
> TPM2_Get_Capability(). If a bank is not allocated, the TPM returns that
> bank in TPML_PCR_SELECTION, with all bits in the mask set to zero. In this
> case, the bank is not included in chip->allocated_banks, to avoid that TPM
> driver users unnecessarily calculate a digest for that bank.
> 
> One PCR bank with algorithm set to SHA1 is always allocated for TPM 1.x.
> 
> As a consequence of the introduction of nr_allocated_banks,
> tpm_pcr_extend() does not check anymore if the algorithm stored in tpm_chip
> is equal to zero.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  drivers/char/tpm/tpm-chip.c      |  1 +
>  drivers/char/tpm/tpm-interface.c | 18 +++++++++--------
>  drivers/char/tpm/tpm.h           |  3 ++-
>  drivers/char/tpm/tpm1-cmd.c      | 10 ++++++++++
>  drivers/char/tpm/tpm2-cmd.c      | 34 ++++++++++++++++++++++----------
>  5 files changed, 47 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 32db84683c40..ce851c62bb68 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -160,6 +160,7 @@ static void tpm_dev_release(struct device *dev)
>  	kfree(chip->log.bios_event_log);
>  	kfree(chip->work_space.context_buf);
>  	kfree(chip->work_space.session_buf);
> +	kfree(chip->allocated_banks);
>  	kfree(chip);
>  }
>  
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index d9439f9abe78..7b80919228be 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -488,8 +488,7 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
>  int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
>  {
>  	int rc;
> -	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> -	u32 count = 0;
> +	struct tpm2_digest *digest_list;
>  	int i;
>  
>  	chip = tpm_find_get_ops(chip);
> @@ -497,16 +496,19 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
>  		return -ENODEV;
>  
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> -		memset(digest_list, 0, sizeof(digest_list));
> +		digest_list = kcalloc(chip->nr_allocated_banks,
> +				      sizeof(*digest_list), GFP_KERNEL);
> +		if (!digest_list)
> +			return -ENOMEM;

You could preallocate digest list and place it to struct tpm_chip
instead of doing it everytime tpm_pcr_extend() called.

>  
> -		for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
> -			    chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
> -			digest_list[i].alg_id = chip->active_banks[i];
> +		for (i = 0; i < chip->nr_allocated_banks; i++) {
> +			digest_list[i].alg_id = chip->allocated_banks[i];
>  			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
> -			count++;
>  		}
>  
> -		rc = tpm2_pcr_extend(chip, pcr_idx, count, digest_list);
> +		rc = tpm2_pcr_extend(chip, pcr_idx, chip->nr_allocated_banks,
> +				     digest_list);
> +		kfree(digest_list);
>  		tpm_put_ops(chip);
>  		return rc;
>  	}
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index f27d1f38a93d..6b94306ab7c5 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -257,7 +257,8 @@ struct tpm_chip {
>  	const struct attribute_group *groups[3];
>  	unsigned int groups_cnt;
>  
> -	u16 active_banks[7];
> +	u32 nr_allocated_banks;
> +	u16 *allocated_banks;
>  #ifdef CONFIG_ACPI
>  	acpi_handle acpi_dev_handle;
>  	char ppi_version[TPM_PPI_VERSION_LEN + 1];
> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> index 6f306338953b..0874743ca96d 100644
> --- a/drivers/char/tpm/tpm1-cmd.c
> +++ b/drivers/char/tpm/tpm1-cmd.c
> @@ -709,6 +709,16 @@ int tpm1_auto_startup(struct tpm_chip *chip)
>  		goto out;
>  	}
>  
> +	chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
> +					GFP_KERNEL);
> +	if (!chip->allocated_banks) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
> +	chip->allocated_banks[0] = TPM2_ALG_SHA1;
> +	chip->nr_allocated_banks = 1;
> +
>  	return rc;
>  out:
>  	if (rc > 0)
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index a6bec13afa69..245669a7aba5 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -234,7 +234,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>  	int i;
>  	int j;
>  
> -	if (count > ARRAY_SIZE(chip->active_banks))
> +	if (count > chip->nr_allocated_banks)
>  		return -EINVAL;
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
> @@ -825,11 +825,14 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>  	void *marker;
>  	void *end;
>  	void *pcr_select_offset;
> -	unsigned int count;
>  	u32 sizeof_pcr_selection;
> +	u32 nr_possible_banks;
> +	u32 nr_alloc_banks = 0;
> +	u16 hash_alg;
>  	u32 rsp_len;
>  	int rc;
>  	int i = 0;
> +	int j;
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
>  	if (rc)
> @@ -844,11 +847,14 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>  	if (rc)
>  		goto out;
>  
> -	count = be32_to_cpup(
> +	nr_possible_banks = be32_to_cpup(
>  		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
>  
> -	if (count > ARRAY_SIZE(chip->active_banks)) {
> -		rc = -ENODEV;
> +	chip->allocated_banks = kcalloc(nr_possible_banks,
> +					sizeof(*chip->allocated_banks),
> +					GFP_KERNEL);
> +	if (!chip->allocated_banks) {
> +		rc = -ENOMEM;
>  		goto out;
>  	}
>  
> @@ -857,7 +863,7 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>  	rsp_len = be32_to_cpup((__be32 *)&buf.data[2]);
>  	end = &buf.data[rsp_len];
>  
> -	for (i = 0; i < count; i++) {
> +	for (i = 0; i < nr_possible_banks; i++) {
>  		pcr_select_offset = marker +
>  			offsetof(struct tpm2_pcr_selection, size_of_select);
>  		if (pcr_select_offset >= end) {
> @@ -866,17 +872,25 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>  		}
>  
>  		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
> -		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
> +		hash_alg = be16_to_cpu(pcr_selection.hash_alg);
> +
> +		for (j = 0; j < pcr_selection.size_of_select; j++)
> +			if (pcr_selection.pcr_select[j])
> +				break;
> +
> +		if (j < pcr_selection.size_of_select) {
> +			chip->allocated_banks[nr_alloc_banks] = hash_alg;
> +			nr_alloc_banks++;
> +		}

The same comment as before: you should use memchr_inv() here.

> +
>  		sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +
>  			sizeof(pcr_selection.size_of_select) +
>  			pcr_selection.size_of_select;
>  		marker = marker + sizeof_pcr_selection;
>  	}
>  
> +	chip->nr_allocated_banks = nr_alloc_banks;
>  out:
> -	if (i < ARRAY_SIZE(chip->active_banks))
> -		chip->active_banks[i] = TPM2_ALG_ERROR;
> -
>  	tpm_buf_destroy(&buf);
>  
>  	return rc;
> -- 
> 2.17.1
> 

/Jarkko
Roberto Sassu Dec. 21, 2018, 9:40 a.m. UTC | #2
On 12/20/2018 3:55 PM, Jarkko Sakkinen wrote:
> On Thu, Dec 13, 2018 at 11:29:41AM +0100, Roberto Sassu wrote:
>> This patch renames active_banks (member of tpm_chip) to allocated_banks,
>> stores the number of allocated PCR banks in nr_allocated_banks (new member
>> of tpm_chip), and replaces the static array with a pointer to a dynamically
>> allocated array.
>>
>> tpm2_get_pcr_allocation() determines if a PCR bank is allocated by checking
>> the mask in the TPML_PCR_SELECTION structure returned by the TPM for
>> TPM2_Get_Capability(). If a bank is not allocated, the TPM returns that
>> bank in TPML_PCR_SELECTION, with all bits in the mask set to zero. In this
>> case, the bank is not included in chip->allocated_banks, to avoid that TPM
>> driver users unnecessarily calculate a digest for that bank.
>>
>> One PCR bank with algorithm set to SHA1 is always allocated for TPM 1.x.
>>
>> As a consequence of the introduction of nr_allocated_banks,
>> tpm_pcr_extend() does not check anymore if the algorithm stored in tpm_chip
>> is equal to zero.
>>
>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>> ---
>>   drivers/char/tpm/tpm-chip.c      |  1 +
>>   drivers/char/tpm/tpm-interface.c | 18 +++++++++--------
>>   drivers/char/tpm/tpm.h           |  3 ++-
>>   drivers/char/tpm/tpm1-cmd.c      | 10 ++++++++++
>>   drivers/char/tpm/tpm2-cmd.c      | 34 ++++++++++++++++++++++----------
>>   5 files changed, 47 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index 32db84683c40..ce851c62bb68 100644
>> --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -160,6 +160,7 @@ static void tpm_dev_release(struct device *dev)
>>   	kfree(chip->log.bios_event_log);
>>   	kfree(chip->work_space.context_buf);
>>   	kfree(chip->work_space.session_buf);
>> +	kfree(chip->allocated_banks);
>>   	kfree(chip);
>>   }
>>   
>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> index d9439f9abe78..7b80919228be 100644
>> --- a/drivers/char/tpm/tpm-interface.c
>> +++ b/drivers/char/tpm/tpm-interface.c
>> @@ -488,8 +488,7 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
>>   int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
>>   {
>>   	int rc;
>> -	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
>> -	u32 count = 0;
>> +	struct tpm2_digest *digest_list;
>>   	int i;
>>   
>>   	chip = tpm_find_get_ops(chip);
>> @@ -497,16 +496,19 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
>>   		return -ENODEV;
>>   
>>   	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>> -		memset(digest_list, 0, sizeof(digest_list));
>> +		digest_list = kcalloc(chip->nr_allocated_banks,
>> +				      sizeof(*digest_list), GFP_KERNEL);
>> +		if (!digest_list)
>> +			return -ENOMEM;
> 
> You could preallocate digest list and place it to struct tpm_chip
> instead of doing it everytime tpm_pcr_extend() called.

This part will be removed with patch 5/5.


>> -		for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
>> -			    chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
>> -			digest_list[i].alg_id = chip->active_banks[i];
>> +		for (i = 0; i < chip->nr_allocated_banks; i++) {
>> +			digest_list[i].alg_id = chip->allocated_banks[i];
>>   			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
>> -			count++;
>>   		}
>>   
>> -		rc = tpm2_pcr_extend(chip, pcr_idx, count, digest_list);
>> +		rc = tpm2_pcr_extend(chip, pcr_idx, chip->nr_allocated_banks,
>> +				     digest_list);
>> +		kfree(digest_list);
>>   		tpm_put_ops(chip);
>>   		return rc;
>>   	}
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index f27d1f38a93d..6b94306ab7c5 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -257,7 +257,8 @@ struct tpm_chip {
>>   	const struct attribute_group *groups[3];
>>   	unsigned int groups_cnt;
>>   
>> -	u16 active_banks[7];
>> +	u32 nr_allocated_banks;
>> +	u16 *allocated_banks;
>>   #ifdef CONFIG_ACPI
>>   	acpi_handle acpi_dev_handle;
>>   	char ppi_version[TPM_PPI_VERSION_LEN + 1];
>> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
>> index 6f306338953b..0874743ca96d 100644
>> --- a/drivers/char/tpm/tpm1-cmd.c
>> +++ b/drivers/char/tpm/tpm1-cmd.c
>> @@ -709,6 +709,16 @@ int tpm1_auto_startup(struct tpm_chip *chip)
>>   		goto out;
>>   	}
>>   
>> +	chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
>> +					GFP_KERNEL);
>> +	if (!chip->allocated_banks) {
>> +		rc = -ENOMEM;
>> +		goto out;
>> +	}
>> +
>> +	chip->allocated_banks[0] = TPM2_ALG_SHA1;
>> +	chip->nr_allocated_banks = 1;
>> +
>>   	return rc;
>>   out:
>>   	if (rc > 0)
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index a6bec13afa69..245669a7aba5 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -234,7 +234,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>>   	int i;
>>   	int j;
>>   
>> -	if (count > ARRAY_SIZE(chip->active_banks))
>> +	if (count > chip->nr_allocated_banks)
>>   		return -EINVAL;
>>   
>>   	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
>> @@ -825,11 +825,14 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>>   	void *marker;
>>   	void *end;
>>   	void *pcr_select_offset;
>> -	unsigned int count;
>>   	u32 sizeof_pcr_selection;
>> +	u32 nr_possible_banks;
>> +	u32 nr_alloc_banks = 0;
>> +	u16 hash_alg;
>>   	u32 rsp_len;
>>   	int rc;
>>   	int i = 0;
>> +	int j;
>>   
>>   	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
>>   	if (rc)
>> @@ -844,11 +847,14 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>>   	if (rc)
>>   		goto out;
>>   
>> -	count = be32_to_cpup(
>> +	nr_possible_banks = be32_to_cpup(
>>   		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
>>   
>> -	if (count > ARRAY_SIZE(chip->active_banks)) {
>> -		rc = -ENODEV;
>> +	chip->allocated_banks = kcalloc(nr_possible_banks,
>> +					sizeof(*chip->allocated_banks),
>> +					GFP_KERNEL);
>> +	if (!chip->allocated_banks) {
>> +		rc = -ENOMEM;
>>   		goto out;
>>   	}
>>   
>> @@ -857,7 +863,7 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>>   	rsp_len = be32_to_cpup((__be32 *)&buf.data[2]);
>>   	end = &buf.data[rsp_len];
>>   
>> -	for (i = 0; i < count; i++) {
>> +	for (i = 0; i < nr_possible_banks; i++) {
>>   		pcr_select_offset = marker +
>>   			offsetof(struct tpm2_pcr_selection, size_of_select);
>>   		if (pcr_select_offset >= end) {
>> @@ -866,17 +872,25 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>>   		}
>>   
>>   		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
>> -		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
>> +		hash_alg = be16_to_cpu(pcr_selection.hash_alg);
>> +
>> +		for (j = 0; j < pcr_selection.size_of_select; j++)
>> +			if (pcr_selection.pcr_select[j])
>> +				break;
>> +
>> +		if (j < pcr_selection.size_of_select) {
>> +			chip->allocated_banks[nr_alloc_banks] = hash_alg;
>> +			nr_alloc_banks++;
>> +		}
> 
> The same comment as before: you should use memchr_inv() here.

Ok.

Roberto


>> +
>>   		sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +
>>   			sizeof(pcr_selection.size_of_select) +
>>   			pcr_selection.size_of_select;
>>   		marker = marker + sizeof_pcr_selection;
>>   	}
>>   
>> +	chip->nr_allocated_banks = nr_alloc_banks;
>>   out:
>> -	if (i < ARRAY_SIZE(chip->active_banks))
>> -		chip->active_banks[i] = TPM2_ALG_ERROR;
>> -
>>   	tpm_buf_destroy(&buf);
>>   
>>   	return rc;
>> -- 
>> 2.17.1
>>
> 
> /Jarkko
>
Jarkko Sakkinen Dec. 22, 2018, 12:03 a.m. UTC | #3
On Fri, Dec 21, 2018 at 10:40:09AM +0100, Roberto Sassu wrote:
> On 12/20/2018 3:55 PM, Jarkko Sakkinen wrote:
> > On Thu, Dec 13, 2018 at 11:29:41AM +0100, Roberto Sassu wrote:
> > > This patch renames active_banks (member of tpm_chip) to allocated_banks,
> > > stores the number of allocated PCR banks in nr_allocated_banks (new member
> > > of tpm_chip), and replaces the static array with a pointer to a dynamically
> > > allocated array.
> > > 
> > > tpm2_get_pcr_allocation() determines if a PCR bank is allocated by checking
> > > the mask in the TPML_PCR_SELECTION structure returned by the TPM for
> > > TPM2_Get_Capability(). If a bank is not allocated, the TPM returns that
> > > bank in TPML_PCR_SELECTION, with all bits in the mask set to zero. In this
> > > case, the bank is not included in chip->allocated_banks, to avoid that TPM
> > > driver users unnecessarily calculate a digest for that bank.
> > > 
> > > One PCR bank with algorithm set to SHA1 is always allocated for TPM 1.x.
> > > 
> > > As a consequence of the introduction of nr_allocated_banks,
> > > tpm_pcr_extend() does not check anymore if the algorithm stored in tpm_chip
> > > is equal to zero.
> > > 
> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > ---
> > >   drivers/char/tpm/tpm-chip.c      |  1 +
> > >   drivers/char/tpm/tpm-interface.c | 18 +++++++++--------
> > >   drivers/char/tpm/tpm.h           |  3 ++-
> > >   drivers/char/tpm/tpm1-cmd.c      | 10 ++++++++++
> > >   drivers/char/tpm/tpm2-cmd.c      | 34 ++++++++++++++++++++++----------
> > >   5 files changed, 47 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > index 32db84683c40..ce851c62bb68 100644
> > > --- a/drivers/char/tpm/tpm-chip.c
> > > +++ b/drivers/char/tpm/tpm-chip.c
> > > @@ -160,6 +160,7 @@ static void tpm_dev_release(struct device *dev)
> > >   	kfree(chip->log.bios_event_log);
> > >   	kfree(chip->work_space.context_buf);
> > >   	kfree(chip->work_space.session_buf);
> > > +	kfree(chip->allocated_banks);
> > >   	kfree(chip);
> > >   }
> > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > > index d9439f9abe78..7b80919228be 100644
> > > --- a/drivers/char/tpm/tpm-interface.c
> > > +++ b/drivers/char/tpm/tpm-interface.c
> > > @@ -488,8 +488,7 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
> > >   int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
> > >   {
> > >   	int rc;
> > > -	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> > > -	u32 count = 0;
> > > +	struct tpm2_digest *digest_list;
> > >   	int i;
> > >   	chip = tpm_find_get_ops(chip);
> > > @@ -497,16 +496,19 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
> > >   		return -ENODEV;
> > >   	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> > > -		memset(digest_list, 0, sizeof(digest_list));
> > > +		digest_list = kcalloc(chip->nr_allocated_banks,
> > > +				      sizeof(*digest_list), GFP_KERNEL);
> > > +		if (!digest_list)
> > > +			return -ENOMEM;
> > 
> > You could preallocate digest list and place it to struct tpm_chip
> > instead of doing it everytime tpm_pcr_extend() called.
> 
> This part will be removed with patch 5/5.

Even if it did, it does not make this patch unbroken.

/Jarkko
Roberto Sassu Jan. 7, 2019, 10:06 a.m. UTC | #4
On 12/22/2018 1:03 AM, Jarkko Sakkinen wrote:
> On Fri, Dec 21, 2018 at 10:40:09AM +0100, Roberto Sassu wrote:
>> On 12/20/2018 3:55 PM, Jarkko Sakkinen wrote:
>>> On Thu, Dec 13, 2018 at 11:29:41AM +0100, Roberto Sassu wrote:
>>>> This patch renames active_banks (member of tpm_chip) to allocated_banks,
>>>> stores the number of allocated PCR banks in nr_allocated_banks (new member
>>>> of tpm_chip), and replaces the static array with a pointer to a dynamically
>>>> allocated array.
>>>>
>>>> tpm2_get_pcr_allocation() determines if a PCR bank is allocated by checking
>>>> the mask in the TPML_PCR_SELECTION structure returned by the TPM for
>>>> TPM2_Get_Capability(). If a bank is not allocated, the TPM returns that
>>>> bank in TPML_PCR_SELECTION, with all bits in the mask set to zero. In this
>>>> case, the bank is not included in chip->allocated_banks, to avoid that TPM
>>>> driver users unnecessarily calculate a digest for that bank.
>>>>
>>>> One PCR bank with algorithm set to SHA1 is always allocated for TPM 1.x.
>>>>
>>>> As a consequence of the introduction of nr_allocated_banks,
>>>> tpm_pcr_extend() does not check anymore if the algorithm stored in tpm_chip
>>>> is equal to zero.
>>>>
>>>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>>>> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>>> ---
>>>>    drivers/char/tpm/tpm-chip.c      |  1 +
>>>>    drivers/char/tpm/tpm-interface.c | 18 +++++++++--------
>>>>    drivers/char/tpm/tpm.h           |  3 ++-
>>>>    drivers/char/tpm/tpm1-cmd.c      | 10 ++++++++++
>>>>    drivers/char/tpm/tpm2-cmd.c      | 34 ++++++++++++++++++++++----------
>>>>    5 files changed, 47 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>>>> index 32db84683c40..ce851c62bb68 100644
>>>> --- a/drivers/char/tpm/tpm-chip.c
>>>> +++ b/drivers/char/tpm/tpm-chip.c
>>>> @@ -160,6 +160,7 @@ static void tpm_dev_release(struct device *dev)
>>>>    	kfree(chip->log.bios_event_log);
>>>>    	kfree(chip->work_space.context_buf);
>>>>    	kfree(chip->work_space.session_buf);
>>>> +	kfree(chip->allocated_banks);
>>>>    	kfree(chip);
>>>>    }
>>>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>>>> index d9439f9abe78..7b80919228be 100644
>>>> --- a/drivers/char/tpm/tpm-interface.c
>>>> +++ b/drivers/char/tpm/tpm-interface.c
>>>> @@ -488,8 +488,7 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
>>>>    int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
>>>>    {
>>>>    	int rc;
>>>> -	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
>>>> -	u32 count = 0;
>>>> +	struct tpm2_digest *digest_list;
>>>>    	int i;
>>>>    	chip = tpm_find_get_ops(chip);
>>>> @@ -497,16 +496,19 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
>>>>    		return -ENODEV;
>>>>    	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>>>> -		memset(digest_list, 0, sizeof(digest_list));
>>>> +		digest_list = kcalloc(chip->nr_allocated_banks,
>>>> +				      sizeof(*digest_list), GFP_KERNEL);
>>>> +		if (!digest_list)
>>>> +			return -ENOMEM;
>>>
>>> You could preallocate digest list and place it to struct tpm_chip
>>> instead of doing it everytime tpm_pcr_extend() called.
>>
>> This part will be removed with patch 5/5.
> 
> Even if it did, it does not make this patch unbroken.

Can two calls to tpm_pcr_extend() be executed at the same time?

If yes, the digest list should be protected by a mutex.

Roberto


> /Jarkko
>
Jarkko Sakkinen Jan. 10, 2019, 5:38 p.m. UTC | #5
On Mon, Jan 07, 2019 at 11:06:33AM +0100, Roberto Sassu wrote:
> On 12/22/2018 1:03 AM, Jarkko Sakkinen wrote:
> > On Fri, Dec 21, 2018 at 10:40:09AM +0100, Roberto Sassu wrote:
> > > On 12/20/2018 3:55 PM, Jarkko Sakkinen wrote:
> > > > On Thu, Dec 13, 2018 at 11:29:41AM +0100, Roberto Sassu wrote:
> > > > > This patch renames active_banks (member of tpm_chip) to allocated_banks,
> > > > > stores the number of allocated PCR banks in nr_allocated_banks (new member
> > > > > of tpm_chip), and replaces the static array with a pointer to a dynamically
> > > > > allocated array.
> > > > > 
> > > > > tpm2_get_pcr_allocation() determines if a PCR bank is allocated by checking
> > > > > the mask in the TPML_PCR_SELECTION structure returned by the TPM for
> > > > > TPM2_Get_Capability(). If a bank is not allocated, the TPM returns that
> > > > > bank in TPML_PCR_SELECTION, with all bits in the mask set to zero. In this
> > > > > case, the bank is not included in chip->allocated_banks, to avoid that TPM
> > > > > driver users unnecessarily calculate a digest for that bank.
> > > > > 
> > > > > One PCR bank with algorithm set to SHA1 is always allocated for TPM 1.x.
> > > > > 
> > > > > As a consequence of the introduction of nr_allocated_banks,
> > > > > tpm_pcr_extend() does not check anymore if the algorithm stored in tpm_chip
> > > > > is equal to zero.
> > > > > 
> > > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > > ---
> > > > >    drivers/char/tpm/tpm-chip.c      |  1 +
> > > > >    drivers/char/tpm/tpm-interface.c | 18 +++++++++--------
> > > > >    drivers/char/tpm/tpm.h           |  3 ++-
> > > > >    drivers/char/tpm/tpm1-cmd.c      | 10 ++++++++++
> > > > >    drivers/char/tpm/tpm2-cmd.c      | 34 ++++++++++++++++++++++----------
> > > > >    5 files changed, 47 insertions(+), 19 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > > > index 32db84683c40..ce851c62bb68 100644
> > > > > --- a/drivers/char/tpm/tpm-chip.c
> > > > > +++ b/drivers/char/tpm/tpm-chip.c
> > > > > @@ -160,6 +160,7 @@ static void tpm_dev_release(struct device *dev)
> > > > >    	kfree(chip->log.bios_event_log);
> > > > >    	kfree(chip->work_space.context_buf);
> > > > >    	kfree(chip->work_space.session_buf);
> > > > > +	kfree(chip->allocated_banks);
> > > > >    	kfree(chip);
> > > > >    }
> > > > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > > > > index d9439f9abe78..7b80919228be 100644
> > > > > --- a/drivers/char/tpm/tpm-interface.c
> > > > > +++ b/drivers/char/tpm/tpm-interface.c
> > > > > @@ -488,8 +488,7 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
> > > > >    int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
> > > > >    {
> > > > >    	int rc;
> > > > > -	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> > > > > -	u32 count = 0;
> > > > > +	struct tpm2_digest *digest_list;
> > > > >    	int i;
> > > > >    	chip = tpm_find_get_ops(chip);
> > > > > @@ -497,16 +496,19 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
> > > > >    		return -ENODEV;
> > > > >    	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> > > > > -		memset(digest_list, 0, sizeof(digest_list));
> > > > > +		digest_list = kcalloc(chip->nr_allocated_banks,
> > > > > +				      sizeof(*digest_list), GFP_KERNEL);
> > > > > +		if (!digest_list)
> > > > > +			return -ENOMEM;
> > > > 
> > > > You could preallocate digest list and place it to struct tpm_chip
> > > > instead of doing it everytime tpm_pcr_extend() called.
> > > 
> > > This part will be removed with patch 5/5.
> > 
> > Even if it did, it does not make this patch unbroken.
> 
> Can two calls to tpm_pcr_extend() be executed at the same time?
> 
> If yes, the digest list should be protected by a mutex.

Good question: the answer is no. Mutex locking is done inside the
transmit flow ATM.

/Jarkko
Roberto Sassu Jan. 11, 2019, 7:53 a.m. UTC | #6
On 1/10/2019 6:38 PM, Jarkko Sakkinen wrote:
> On Mon, Jan 07, 2019 at 11:06:33AM +0100, Roberto Sassu wrote:
>> On 12/22/2018 1:03 AM, Jarkko Sakkinen wrote:
>>> On Fri, Dec 21, 2018 at 10:40:09AM +0100, Roberto Sassu wrote:
>>>> On 12/20/2018 3:55 PM, Jarkko Sakkinen wrote:
>>>>> On Thu, Dec 13, 2018 at 11:29:41AM +0100, Roberto Sassu wrote:
>>>>>> This patch renames active_banks (member of tpm_chip) to allocated_banks,
>>>>>> stores the number of allocated PCR banks in nr_allocated_banks (new member
>>>>>> of tpm_chip), and replaces the static array with a pointer to a dynamically
>>>>>> allocated array.
>>>>>>
>>>>>> tpm2_get_pcr_allocation() determines if a PCR bank is allocated by checking
>>>>>> the mask in the TPML_PCR_SELECTION structure returned by the TPM for
>>>>>> TPM2_Get_Capability(). If a bank is not allocated, the TPM returns that
>>>>>> bank in TPML_PCR_SELECTION, with all bits in the mask set to zero. In this
>>>>>> case, the bank is not included in chip->allocated_banks, to avoid that TPM
>>>>>> driver users unnecessarily calculate a digest for that bank.
>>>>>>
>>>>>> One PCR bank with algorithm set to SHA1 is always allocated for TPM 1.x.
>>>>>>
>>>>>> As a consequence of the introduction of nr_allocated_banks,
>>>>>> tpm_pcr_extend() does not check anymore if the algorithm stored in tpm_chip
>>>>>> is equal to zero.
>>>>>>
>>>>>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>>>>>> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>>>>> ---
>>>>>>     drivers/char/tpm/tpm-chip.c      |  1 +
>>>>>>     drivers/char/tpm/tpm-interface.c | 18 +++++++++--------
>>>>>>     drivers/char/tpm/tpm.h           |  3 ++-
>>>>>>     drivers/char/tpm/tpm1-cmd.c      | 10 ++++++++++
>>>>>>     drivers/char/tpm/tpm2-cmd.c      | 34 ++++++++++++++++++++++----------
>>>>>>     5 files changed, 47 insertions(+), 19 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>>>>>> index 32db84683c40..ce851c62bb68 100644
>>>>>> --- a/drivers/char/tpm/tpm-chip.c
>>>>>> +++ b/drivers/char/tpm/tpm-chip.c
>>>>>> @@ -160,6 +160,7 @@ static void tpm_dev_release(struct device *dev)
>>>>>>     	kfree(chip->log.bios_event_log);
>>>>>>     	kfree(chip->work_space.context_buf);
>>>>>>     	kfree(chip->work_space.session_buf);
>>>>>> +	kfree(chip->allocated_banks);
>>>>>>     	kfree(chip);
>>>>>>     }
>>>>>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>>>>>> index d9439f9abe78..7b80919228be 100644
>>>>>> --- a/drivers/char/tpm/tpm-interface.c
>>>>>> +++ b/drivers/char/tpm/tpm-interface.c
>>>>>> @@ -488,8 +488,7 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
>>>>>>     int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
>>>>>>     {
>>>>>>     	int rc;
>>>>>> -	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
>>>>>> -	u32 count = 0;
>>>>>> +	struct tpm2_digest *digest_list;
>>>>>>     	int i;
>>>>>>     	chip = tpm_find_get_ops(chip);
>>>>>> @@ -497,16 +496,19 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
>>>>>>     		return -ENODEV;
>>>>>>     	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>>>>>> -		memset(digest_list, 0, sizeof(digest_list));
>>>>>> +		digest_list = kcalloc(chip->nr_allocated_banks,
>>>>>> +				      sizeof(*digest_list), GFP_KERNEL);
>>>>>> +		if (!digest_list)
>>>>>> +			return -ENOMEM;
>>>>>
>>>>> You could preallocate digest list and place it to struct tpm_chip
>>>>> instead of doing it everytime tpm_pcr_extend() called.
>>>>
>>>> This part will be removed with patch 5/5.
>>>
>>> Even if it did, it does not make this patch unbroken.
>>
>> Can two calls to tpm_pcr_extend() be executed at the same time?
>>
>> If yes, the digest list should be protected by a mutex.
> 
> Good question: the answer is no. Mutex locking is done inside the
> transmit flow ATM.

But data are copied before the mutex is locked. Can't a second call
overwrite chip->preallocated_digest_list while the first call is still
writing it?

Roberto


> /Jarkko
>
Jarkko Sakkinen Jan. 11, 2019, 4:35 p.m. UTC | #7
On Fri, Jan 11, 2019 at 08:53:00AM +0100, Roberto Sassu wrote:
> On 1/10/2019 6:38 PM, Jarkko Sakkinen wrote:
> > On Mon, Jan 07, 2019 at 11:06:33AM +0100, Roberto Sassu wrote:
> > > On 12/22/2018 1:03 AM, Jarkko Sakkinen wrote:
> > > > On Fri, Dec 21, 2018 at 10:40:09AM +0100, Roberto Sassu wrote:
> > > > > On 12/20/2018 3:55 PM, Jarkko Sakkinen wrote:
> > > > > > On Thu, Dec 13, 2018 at 11:29:41AM +0100, Roberto Sassu wrote:
> > > > > > > This patch renames active_banks (member of tpm_chip) to allocated_banks,
> > > > > > > stores the number of allocated PCR banks in nr_allocated_banks (new member
> > > > > > > of tpm_chip), and replaces the static array with a pointer to a dynamically
> > > > > > > allocated array.
> > > > > > > 
> > > > > > > tpm2_get_pcr_allocation() determines if a PCR bank is allocated by checking
> > > > > > > the mask in the TPML_PCR_SELECTION structure returned by the TPM for
> > > > > > > TPM2_Get_Capability(). If a bank is not allocated, the TPM returns that
> > > > > > > bank in TPML_PCR_SELECTION, with all bits in the mask set to zero. In this
> > > > > > > case, the bank is not included in chip->allocated_banks, to avoid that TPM
> > > > > > > driver users unnecessarily calculate a digest for that bank.
> > > > > > > 
> > > > > > > One PCR bank with algorithm set to SHA1 is always allocated for TPM 1.x.
> > > > > > > 
> > > > > > > As a consequence of the introduction of nr_allocated_banks,
> > > > > > > tpm_pcr_extend() does not check anymore if the algorithm stored in tpm_chip
> > > > > > > is equal to zero.
> > > > > > > 
> > > > > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > > > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > > > > ---
> > > > > > >     drivers/char/tpm/tpm-chip.c      |  1 +
> > > > > > >     drivers/char/tpm/tpm-interface.c | 18 +++++++++--------
> > > > > > >     drivers/char/tpm/tpm.h           |  3 ++-
> > > > > > >     drivers/char/tpm/tpm1-cmd.c      | 10 ++++++++++
> > > > > > >     drivers/char/tpm/tpm2-cmd.c      | 34 ++++++++++++++++++++++----------
> > > > > > >     5 files changed, 47 insertions(+), 19 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > > > > > index 32db84683c40..ce851c62bb68 100644
> > > > > > > --- a/drivers/char/tpm/tpm-chip.c
> > > > > > > +++ b/drivers/char/tpm/tpm-chip.c
> > > > > > > @@ -160,6 +160,7 @@ static void tpm_dev_release(struct device *dev)
> > > > > > >     	kfree(chip->log.bios_event_log);
> > > > > > >     	kfree(chip->work_space.context_buf);
> > > > > > >     	kfree(chip->work_space.session_buf);
> > > > > > > +	kfree(chip->allocated_banks);
> > > > > > >     	kfree(chip);
> > > > > > >     }
> > > > > > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > > > > > > index d9439f9abe78..7b80919228be 100644
> > > > > > > --- a/drivers/char/tpm/tpm-interface.c
> > > > > > > +++ b/drivers/char/tpm/tpm-interface.c
> > > > > > > @@ -488,8 +488,7 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
> > > > > > >     int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
> > > > > > >     {
> > > > > > >     	int rc;
> > > > > > > -	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> > > > > > > -	u32 count = 0;
> > > > > > > +	struct tpm2_digest *digest_list;
> > > > > > >     	int i;
> > > > > > >     	chip = tpm_find_get_ops(chip);
> > > > > > > @@ -497,16 +496,19 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
> > > > > > >     		return -ENODEV;
> > > > > > >     	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> > > > > > > -		memset(digest_list, 0, sizeof(digest_list));
> > > > > > > +		digest_list = kcalloc(chip->nr_allocated_banks,
> > > > > > > +				      sizeof(*digest_list), GFP_KERNEL);
> > > > > > > +		if (!digest_list)
> > > > > > > +			return -ENOMEM;
> > > > > > 
> > > > > > You could preallocate digest list and place it to struct tpm_chip
> > > > > > instead of doing it everytime tpm_pcr_extend() called.
> > > > > 
> > > > > This part will be removed with patch 5/5.
> > > > 
> > > > Even if it did, it does not make this patch unbroken.
> > > 
> > > Can two calls to tpm_pcr_extend() be executed at the same time?
> > > 
> > > If yes, the digest list should be protected by a mutex.
> > 
> > Good question: the answer is no. Mutex locking is done inside the
> > transmit flow ATM.
> 
> But data are copied before the mutex is locked. Can't a second call
> overwrite chip->preallocated_digest_list while the first call is still
> writing it?

Now I see what you mean. I have patch set on review that would make this
more natural to do. Locking can be done now too so that it would work
but that would counter-productive, so lets keep the approach that you
have.

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 32db84683c40..ce851c62bb68 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -160,6 +160,7 @@  static void tpm_dev_release(struct device *dev)
 	kfree(chip->log.bios_event_log);
 	kfree(chip->work_space.context_buf);
 	kfree(chip->work_space.session_buf);
+	kfree(chip->allocated_banks);
 	kfree(chip);
 }
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d9439f9abe78..7b80919228be 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -488,8 +488,7 @@  EXPORT_SYMBOL_GPL(tpm_pcr_read);
 int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
 {
 	int rc;
-	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
-	u32 count = 0;
+	struct tpm2_digest *digest_list;
 	int i;
 
 	chip = tpm_find_get_ops(chip);
@@ -497,16 +496,19 @@  int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
 		return -ENODEV;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		memset(digest_list, 0, sizeof(digest_list));
+		digest_list = kcalloc(chip->nr_allocated_banks,
+				      sizeof(*digest_list), GFP_KERNEL);
+		if (!digest_list)
+			return -ENOMEM;
 
-		for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
-			    chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
-			digest_list[i].alg_id = chip->active_banks[i];
+		for (i = 0; i < chip->nr_allocated_banks; i++) {
+			digest_list[i].alg_id = chip->allocated_banks[i];
 			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
-			count++;
 		}
 
-		rc = tpm2_pcr_extend(chip, pcr_idx, count, digest_list);
+		rc = tpm2_pcr_extend(chip, pcr_idx, chip->nr_allocated_banks,
+				     digest_list);
+		kfree(digest_list);
 		tpm_put_ops(chip);
 		return rc;
 	}
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f27d1f38a93d..6b94306ab7c5 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -257,7 +257,8 @@  struct tpm_chip {
 	const struct attribute_group *groups[3];
 	unsigned int groups_cnt;
 
-	u16 active_banks[7];
+	u32 nr_allocated_banks;
+	u16 *allocated_banks;
 #ifdef CONFIG_ACPI
 	acpi_handle acpi_dev_handle;
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 6f306338953b..0874743ca96d 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -709,6 +709,16 @@  int tpm1_auto_startup(struct tpm_chip *chip)
 		goto out;
 	}
 
+	chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
+					GFP_KERNEL);
+	if (!chip->allocated_banks) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	chip->allocated_banks[0] = TPM2_ALG_SHA1;
+	chip->nr_allocated_banks = 1;
+
 	return rc;
 out:
 	if (rc > 0)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a6bec13afa69..245669a7aba5 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -234,7 +234,7 @@  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
 	int i;
 	int j;
 
-	if (count > ARRAY_SIZE(chip->active_banks))
+	if (count > chip->nr_allocated_banks)
 		return -EINVAL;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
@@ -825,11 +825,14 @@  static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 	void *marker;
 	void *end;
 	void *pcr_select_offset;
-	unsigned int count;
 	u32 sizeof_pcr_selection;
+	u32 nr_possible_banks;
+	u32 nr_alloc_banks = 0;
+	u16 hash_alg;
 	u32 rsp_len;
 	int rc;
 	int i = 0;
+	int j;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
 	if (rc)
@@ -844,11 +847,14 @@  static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 	if (rc)
 		goto out;
 
-	count = be32_to_cpup(
+	nr_possible_banks = be32_to_cpup(
 		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
 
-	if (count > ARRAY_SIZE(chip->active_banks)) {
-		rc = -ENODEV;
+	chip->allocated_banks = kcalloc(nr_possible_banks,
+					sizeof(*chip->allocated_banks),
+					GFP_KERNEL);
+	if (!chip->allocated_banks) {
+		rc = -ENOMEM;
 		goto out;
 	}
 
@@ -857,7 +863,7 @@  static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 	rsp_len = be32_to_cpup((__be32 *)&buf.data[2]);
 	end = &buf.data[rsp_len];
 
-	for (i = 0; i < count; i++) {
+	for (i = 0; i < nr_possible_banks; i++) {
 		pcr_select_offset = marker +
 			offsetof(struct tpm2_pcr_selection, size_of_select);
 		if (pcr_select_offset >= end) {
@@ -866,17 +872,25 @@  static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 		}
 
 		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
-		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
+		hash_alg = be16_to_cpu(pcr_selection.hash_alg);
+
+		for (j = 0; j < pcr_selection.size_of_select; j++)
+			if (pcr_selection.pcr_select[j])
+				break;
+
+		if (j < pcr_selection.size_of_select) {
+			chip->allocated_banks[nr_alloc_banks] = hash_alg;
+			nr_alloc_banks++;
+		}
+
 		sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +
 			sizeof(pcr_selection.size_of_select) +
 			pcr_selection.size_of_select;
 		marker = marker + sizeof_pcr_selection;
 	}
 
+	chip->nr_allocated_banks = nr_alloc_banks;
 out:
-	if (i < ARRAY_SIZE(chip->active_banks))
-		chip->active_banks[i] = TPM2_ALG_ERROR;
-
 	tpm_buf_destroy(&buf);
 
 	return rc;