diff mbox series

[v2,3/3] tpm: retrieve digest size of unknown algorithms with PCR read

Message ID 20180905114202.7757-4-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 Sept. 5, 2018, 11:42 a.m. UTC
Currently, the TPM driver retrieves the digest size from a table mapping
TPM algorithms identifiers to identifiers defined by the crypto subsystem.
If the algorithm is not defined by the latter, the digest size can be
retrieved from the output of the PCR read command.

The patch retrieves at TPM startup the digest sizes for each PCR bank and
stores them in the new structure tpm_active_bank_info, member of tpm_chip,
so that the information can be passed to other kernel subsystems.

tpm_active_bank_info contains: the TPM algorithm identifier, necessary to
generate the event log as defined by Trusted Computing Group (TCG); the
digest size, to pad/truncate a digest calculated with a different
algorithm; the crypto subsystem identifier, to calculate the digest of
event data.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 drivers/char/tpm/tpm-interface.c | 11 +++++++---
 drivers/char/tpm/tpm.h           |  4 ++--
 drivers/char/tpm/tpm2-cmd.c      | 47 ++++++++++++++++++++++++++++++----------
 include/linux/tpm.h              |  6 +++++
 4 files changed, 51 insertions(+), 17 deletions(-)

Comments

Jarkko Sakkinen Sept. 16, 2018, 12:37 p.m. UTC | #1
On Wed, Sep 05, 2018 at 01:42:02PM +0200, Roberto Sassu wrote:
> Currently, the TPM driver retrieves the digest size from a table mapping
> TPM algorithms identifiers to identifiers defined by the crypto subsystem.
> If the algorithm is not defined by the latter, the digest size can be
> retrieved from the output of the PCR read command.
> 
> The patch retrieves at TPM startup the digest sizes for each PCR bank and
> stores them in the new structure tpm_active_bank_info, member of tpm_chip,
> so that the information can be passed to other kernel subsystems.
> 
> tpm_active_bank_info contains: the TPM algorithm identifier, necessary to
> generate the event log as defined by Trusted Computing Group (TCG); the
> digest size, to pad/truncate a digest calculated with a different
> algorithm; the crypto subsystem identifier, to calculate the digest of
> event data.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  drivers/char/tpm/tpm-interface.c | 11 +++++++---
>  drivers/char/tpm/tpm.h           |  4 ++--
>  drivers/char/tpm/tpm2-cmd.c      | 47 ++++++++++++++++++++++++++++++----------
>  include/linux/tpm.h              |  6 +++++
>  4 files changed, 51 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 81872880b5f1..02dcdcda5a3c 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -993,7 +993,7 @@ int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
>  	if (!chip)
>  		return -ENODEV;
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> -		rc = tpm2_pcr_read(chip, pcr_idx, count, digests);
> +		rc = tpm2_pcr_read(chip, pcr_idx, count, digests, NULL);
>  	else
>  		rc = tpm_pcr_read_dev(chip, pcr_idx, digests[0].digest);
>  	tpm_put_ops(chip);
> @@ -1056,8 +1056,8 @@ int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
>  		memset(digest_list, 0, sizeof(digest_list));
>  
>  		for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
> -			    chip->active_banks[i] != TPM_ALG_ERROR; i++) {
> -			digest_list[i].alg_id = chip->active_banks[i];
> +		     chip->active_banks[i].alg_id != TPM_ALG_ERROR; i++) {
> +			digest_list[i].alg_id = chip->active_banks[i].alg_id;

Why not just zero?

>  			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
>  			count++;
>  		}
> @@ -1158,6 +1158,11 @@ int tpm1_auto_startup(struct tpm_chip *chip)
>  		goto out;
>  	}
>  
> +	chip->active_banks[0].alg_id = TPM_ALG_SHA1;
> +	chip->active_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> +	chip->active_banks[0].crypto_id = HASH_ALGO_SHA1;
> +	chip->active_banks[1].alg_id = TPM_ALG_ERROR;

Then you could drop the last line here.

> +
>  	return rc;
>  out:
>  	if (rc > 0)
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 9479e1ae1b4c..385843b49c17 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -237,7 +237,7 @@ struct tpm_chip {
>  	const struct attribute_group *groups[3];
>  	unsigned int groups_cnt;
>  
> -	u16 active_banks[7];
> +	struct tpm_active_bank_info active_banks[7];
>  #ifdef CONFIG_ACPI
>  	acpi_handle acpi_dev_handle;
>  	char ppi_version[TPM_PPI_VERSION_LEN + 1];
> @@ -565,7 +565,7 @@ static inline u32 tpm2_rc_value(u32 rc)
>  }
>  
>  int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
> -		  struct tpm_digest *digests);
> +		  struct tpm_digest *digests, u16 *sizes);
>  int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  		    struct tpm_digest *digests);
>  int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 601d67c76c1e..51d2454c5329 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -181,11 +181,12 @@ struct tpm2_pcr_read_out {
>   * @pcr_idx:	index of the PCR to read.
>   * @count:	number of digests passed.
>   * @digests:	list of pcr banks and buffers current PCR values are written to.
> + * @sizes:	list of digest sizes.
>   *
>   * Return: Same as with tpm_transmit_cmd.
>   */
>  int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
> -		  struct tpm_digest *digests)
> +		  struct tpm_digest *digests, u16 *sizes)
>  {
>  	int rc;
>  	struct tpm_buf buf;
> @@ -215,6 +216,9 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
>  		out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
>  		memcpy(digests[0].digest, out->digest,
>  		       be16_to_cpu(out->digest_size));
> +
> +		if (sizes)
> +			sizes[0] = be16_to_cpu(out->digest_size);
>  	}
>  
>  	tpm_buf_destroy(&buf);
> @@ -245,7 +249,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  	struct tpm2_null_auth_area auth_area;
>  	int rc;
>  	int i;
> -	int j;
>  
>  	if (count > ARRAY_SIZE(chip->active_banks))
>  		return -EINVAL;
> @@ -267,14 +270,9 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  	tpm_buf_append_u32(&buf, count);
>  
>  	for (i = 0; i < count; i++) {
> -		for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) {
> -			if (digests[i].alg_id != tpm2_hash_map[j].tpm_id)
> -				continue;
> -			tpm_buf_append_u16(&buf, digests[i].alg_id);
> -			tpm_buf_append(&buf, (const unsigned char
> -					      *)&digests[i].digest,
> -			       hash_digest_size[tpm2_hash_map[j].crypto_id]);
> -		}
> +		tpm_buf_append_u16(&buf, digests[i].alg_id);
> +		tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
> +			       chip->active_banks[i].digest_size);
>  	}
>  
>  	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
> @@ -851,6 +849,26 @@ int tpm2_probe(struct tpm_chip *chip)
>  }
>  EXPORT_SYMBOL_GPL(tpm2_probe);
>  
> +static int tpm2_init_active_bank_info(struct tpm_chip *chip,
> +				      struct tpm_active_bank_info *active_bank)
> +{
> +	struct tpm_digest digest = {.alg_id = active_bank->alg_id};
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
> +		enum hash_algo crypto_algo = tpm2_hash_map[i].crypto_id;
> +
> +		if (active_bank->alg_id != tpm2_hash_map[i].tpm_id)
> +			continue;
> +
> +		active_bank->digest_size = hash_digest_size[crypto_algo];
> +		active_bank->crypto_id = crypto_algo;
> +		return 0;
> +	}
> +
> +	return tpm2_pcr_read(chip, 0, 1, &digest, &active_bank->digest_size);
> +}
> +
>  struct tpm2_pcr_selection {
>  	__be16  hash_alg;
>  	u8  size_of_select;
> @@ -905,7 +923,12 @@ 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);
> +		chip->active_banks[i].alg_id =
> +			be16_to_cpu(pcr_selection.hash_alg);
> +		rc =  tpm2_init_active_bank_info(chip, &chip->active_banks[i]);
> +		if (rc)
> +			break;
> +
>  		sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +
>  			sizeof(pcr_selection.size_of_select) +
>  			pcr_selection.size_of_select;
> @@ -914,7 +937,7 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>  
>  out:
>  	if (i < ARRAY_SIZE(chip->active_banks))
> -		chip->active_banks[i] = TPM_ALG_ERROR;
> +		chip->active_banks[i].alg_id = TPM_ALG_ERROR;
>  
>  	tpm_buf_destroy(&buf);
>  
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index ac9fc47f4494..c8372ff582c3 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -46,6 +46,12 @@ struct tpm_digest {
>  	u8 digest[SHA512_DIGEST_SIZE];
>  } __packed;
>  
> +struct tpm_active_bank_info {
> +	u16 alg_id;
> +	u16 digest_size;
> +	u16 crypto_id;
> +};

Why not just tpm_bank_info? Will there be also tpm_inactive_bank_info
at some point?

> +
>  enum TPM_OPS_FLAGS {
>  	TPM_OPS_AUTO_STARTUP = BIT(0),
>  };
> -- 
> 2.14.1
> 

/Jarkko
Roberto Sassu Sept. 17, 2018, 10:02 a.m. UTC | #2
On 9/16/2018 2:37 PM, Jarkko Sakkinen wrote:
> On Wed, Sep 05, 2018 at 01:42:02PM +0200, Roberto Sassu wrote:
>> Currently, the TPM driver retrieves the digest size from a table mapping
>> TPM algorithms identifiers to identifiers defined by the crypto subsystem.
>> If the algorithm is not defined by the latter, the digest size can be
>> retrieved from the output of the PCR read command.
>>
>> The patch retrieves at TPM startup the digest sizes for each PCR bank and
>> stores them in the new structure tpm_active_bank_info, member of tpm_chip,
>> so that the information can be passed to other kernel subsystems.
>>
>> tpm_active_bank_info contains: the TPM algorithm identifier, necessary to
>> generate the event log as defined by Trusted Computing Group (TCG); the
>> digest size, to pad/truncate a digest calculated with a different
>> algorithm; the crypto subsystem identifier, to calculate the digest of
>> event data.
>>
>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>> ---
>>   drivers/char/tpm/tpm-interface.c | 11 +++++++---
>>   drivers/char/tpm/tpm.h           |  4 ++--
>>   drivers/char/tpm/tpm2-cmd.c      | 47 ++++++++++++++++++++++++++++++----------
>>   include/linux/tpm.h              |  6 +++++
>>   4 files changed, 51 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> index 81872880b5f1..02dcdcda5a3c 100644
>> --- a/drivers/char/tpm/tpm-interface.c
>> +++ b/drivers/char/tpm/tpm-interface.c
>> @@ -993,7 +993,7 @@ int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
>>   	if (!chip)
>>   		return -ENODEV;
>>   	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> -		rc = tpm2_pcr_read(chip, pcr_idx, count, digests);
>> +		rc = tpm2_pcr_read(chip, pcr_idx, count, digests, NULL);
>>   	else
>>   		rc = tpm_pcr_read_dev(chip, pcr_idx, digests[0].digest);
>>   	tpm_put_ops(chip);
>> @@ -1056,8 +1056,8 @@ int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
>>   		memset(digest_list, 0, sizeof(digest_list));
>>   
>>   		for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
>> -			    chip->active_banks[i] != TPM_ALG_ERROR; i++) {
>> -			digest_list[i].alg_id = chip->active_banks[i];
>> +		     chip->active_banks[i].alg_id != TPM_ALG_ERROR; i++) {
>> +			digest_list[i].alg_id = chip->active_banks[i].alg_id;
> 
> Why not just zero?

Hi Jarkko

sorry, I didn't understand this comment. Did you refer to the code
below (chip->active_banks[0] ...)?


>>   			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
>>   			count++;
>>   		}
>> @@ -1158,6 +1158,11 @@ int tpm1_auto_startup(struct tpm_chip *chip)
>>   		goto out;
>>   	}
>>   
>> +	chip->active_banks[0].alg_id = TPM_ALG_SHA1;
>> +	chip->active_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
>> +	chip->active_banks[0].crypto_id = HASH_ALGO_SHA1;
>> +	chip->active_banks[1].alg_id = TPM_ALG_ERROR;
> 
> Then you could drop the last line here.

This code has the same behavior of tpm2_get_pcr_allocation(). If some
banks are not used, set the algorithm of the first unused to
TPM_ALG_ERROR.


>> +
>>   	return rc;
>>   out:
>>   	if (rc > 0)
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 9479e1ae1b4c..385843b49c17 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -237,7 +237,7 @@ struct tpm_chip {
>>   	const struct attribute_group *groups[3];
>>   	unsigned int groups_cnt;
>>   
>> -	u16 active_banks[7];
>> +	struct tpm_active_bank_info active_banks[7];
>>   #ifdef CONFIG_ACPI
>>   	acpi_handle acpi_dev_handle;
>>   	char ppi_version[TPM_PPI_VERSION_LEN + 1];
>> @@ -565,7 +565,7 @@ static inline u32 tpm2_rc_value(u32 rc)
>>   }
>>   
>>   int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
>> -		  struct tpm_digest *digests);
>> +		  struct tpm_digest *digests, u16 *sizes);
>>   int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>>   		    struct tpm_digest *digests);
>>   int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 601d67c76c1e..51d2454c5329 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -181,11 +181,12 @@ struct tpm2_pcr_read_out {
>>    * @pcr_idx:	index of the PCR to read.
>>    * @count:	number of digests passed.
>>    * @digests:	list of pcr banks and buffers current PCR values are written to.
>> + * @sizes:	list of digest sizes.
>>    *
>>    * Return: Same as with tpm_transmit_cmd.
>>    */
>>   int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
>> -		  struct tpm_digest *digests)
>> +		  struct tpm_digest *digests, u16 *sizes)
>>   {
>>   	int rc;
>>   	struct tpm_buf buf;
>> @@ -215,6 +216,9 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
>>   		out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
>>   		memcpy(digests[0].digest, out->digest,
>>   		       be16_to_cpu(out->digest_size));
>> +
>> +		if (sizes)
>> +			sizes[0] = be16_to_cpu(out->digest_size);
>>   	}
>>   
>>   	tpm_buf_destroy(&buf);
>> @@ -245,7 +249,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>>   	struct tpm2_null_auth_area auth_area;
>>   	int rc;
>>   	int i;
>> -	int j;
>>   
>>   	if (count > ARRAY_SIZE(chip->active_banks))
>>   		return -EINVAL;
>> @@ -267,14 +270,9 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>>   	tpm_buf_append_u32(&buf, count);
>>   
>>   	for (i = 0; i < count; i++) {
>> -		for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) {
>> -			if (digests[i].alg_id != tpm2_hash_map[j].tpm_id)
>> -				continue;
>> -			tpm_buf_append_u16(&buf, digests[i].alg_id);
>> -			tpm_buf_append(&buf, (const unsigned char
>> -					      *)&digests[i].digest,
>> -			       hash_digest_size[tpm2_hash_map[j].crypto_id]);
>> -		}
>> +		tpm_buf_append_u16(&buf, digests[i].alg_id);
>> +		tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
>> +			       chip->active_banks[i].digest_size);
>>   	}
>>   
>>   	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
>> @@ -851,6 +849,26 @@ int tpm2_probe(struct tpm_chip *chip)
>>   }
>>   EXPORT_SYMBOL_GPL(tpm2_probe);
>>   
>> +static int tpm2_init_active_bank_info(struct tpm_chip *chip,
>> +				      struct tpm_active_bank_info *active_bank)
>> +{
>> +	struct tpm_digest digest = {.alg_id = active_bank->alg_id};
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
>> +		enum hash_algo crypto_algo = tpm2_hash_map[i].crypto_id;
>> +
>> +		if (active_bank->alg_id != tpm2_hash_map[i].tpm_id)
>> +			continue;
>> +
>> +		active_bank->digest_size = hash_digest_size[crypto_algo];
>> +		active_bank->crypto_id = crypto_algo;
>> +		return 0;
>> +	}
>> +
>> +	return tpm2_pcr_read(chip, 0, 1, &digest, &active_bank->digest_size);
>> +}
>> +
>>   struct tpm2_pcr_selection {
>>   	__be16  hash_alg;
>>   	u8  size_of_select;
>> @@ -905,7 +923,12 @@ 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);
>> +		chip->active_banks[i].alg_id =
>> +			be16_to_cpu(pcr_selection.hash_alg);
>> +		rc =  tpm2_init_active_bank_info(chip, &chip->active_banks[i]);
>> +		if (rc)
>> +			break;
>> +
>>   		sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +
>>   			sizeof(pcr_selection.size_of_select) +
>>   			pcr_selection.size_of_select;
>> @@ -914,7 +937,7 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>>   
>>   out:
>>   	if (i < ARRAY_SIZE(chip->active_banks))
>> -		chip->active_banks[i] = TPM_ALG_ERROR;
>> +		chip->active_banks[i].alg_id = TPM_ALG_ERROR;
>>   
>>   	tpm_buf_destroy(&buf);
>>   
>> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>> index ac9fc47f4494..c8372ff582c3 100644
>> --- a/include/linux/tpm.h
>> +++ b/include/linux/tpm.h
>> @@ -46,6 +46,12 @@ struct tpm_digest {
>>   	u8 digest[SHA512_DIGEST_SIZE];
>>   } __packed;
>>   
>> +struct tpm_active_bank_info {
>> +	u16 alg_id;
>> +	u16 digest_size;
>> +	u16 crypto_id;
>> +};
> 
> Why not just tpm_bank_info? Will there be also tpm_inactive_bank_info
> at some point?

It derived from chip->active_banks. I will rename the structure.

Thanks

Roberto
Jarkko Sakkinen Sept. 17, 2018, 9:16 p.m. UTC | #3
On Mon, Sep 17, 2018 at 12:02:56PM +0200, Roberto Sassu wrote:
> This code has the same behavior of tpm2_get_pcr_allocation(). If some
> banks are not used, set the algorithm of the first unused to
> TPM_ALG_ERROR.

My point is that maybe it would sense to use zero for that in order
to make code a bit simpler.

/Jarkko
Roberto Sassu Sept. 18, 2018, 7:14 a.m. UTC | #4
On 9/17/2018 11:16 PM, Jarkko Sakkinen wrote:
> On Mon, Sep 17, 2018 at 12:02:56PM +0200, Roberto Sassu wrote:
>> This code has the same behavior of tpm2_get_pcr_allocation(). If some
>> banks are not used, set the algorithm of the first unused to
>> TPM_ALG_ERROR.
> 
> My point is that maybe it would sense to use zero for that in order
> to make code a bit simpler.

Wouldn't be better to compare data with the same type? Since the alg_id
structure member stores an algorithm, it should be fine to compare its
value with an algorithm.

No problem to change that, but probably I should modify also
tpm_pcr_extend().

Roberto
Jarkko Sakkinen Sept. 18, 2018, 6:54 p.m. UTC | #5
On Tue, Sep 18, 2018 at 09:14:43AM +0200, Roberto Sassu wrote:
> On 9/17/2018 11:16 PM, Jarkko Sakkinen wrote:
> > On Mon, Sep 17, 2018 at 12:02:56PM +0200, Roberto Sassu wrote:
> > > This code has the same behavior of tpm2_get_pcr_allocation(). If some
> > > banks are not used, set the algorithm of the first unused to
> > > TPM_ALG_ERROR.
> > 
> > My point is that maybe it would sense to use zero for that in order
> > to make code a bit simpler.
> 
> Wouldn't be better to compare data with the same type? Since the alg_id
> structure member stores an algorithm, it should be fine to compare its
> value with an algorithm.
> 
> No problem to change that, but probably I should modify also
> tpm_pcr_extend().
> 
> Roberto

Please do.

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 81872880b5f1..02dcdcda5a3c 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -993,7 +993,7 @@  int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
 	if (!chip)
 		return -ENODEV;
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		rc = tpm2_pcr_read(chip, pcr_idx, count, digests);
+		rc = tpm2_pcr_read(chip, pcr_idx, count, digests, NULL);
 	else
 		rc = tpm_pcr_read_dev(chip, pcr_idx, digests[0].digest);
 	tpm_put_ops(chip);
@@ -1056,8 +1056,8 @@  int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
 		memset(digest_list, 0, sizeof(digest_list));
 
 		for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
-			    chip->active_banks[i] != TPM_ALG_ERROR; i++) {
-			digest_list[i].alg_id = chip->active_banks[i];
+		     chip->active_banks[i].alg_id != TPM_ALG_ERROR; i++) {
+			digest_list[i].alg_id = chip->active_banks[i].alg_id;
 			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
 			count++;
 		}
@@ -1158,6 +1158,11 @@  int tpm1_auto_startup(struct tpm_chip *chip)
 		goto out;
 	}
 
+	chip->active_banks[0].alg_id = TPM_ALG_SHA1;
+	chip->active_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
+	chip->active_banks[0].crypto_id = HASH_ALGO_SHA1;
+	chip->active_banks[1].alg_id = TPM_ALG_ERROR;
+
 	return rc;
 out:
 	if (rc > 0)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 9479e1ae1b4c..385843b49c17 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -237,7 +237,7 @@  struct tpm_chip {
 	const struct attribute_group *groups[3];
 	unsigned int groups_cnt;
 
-	u16 active_banks[7];
+	struct tpm_active_bank_info active_banks[7];
 #ifdef CONFIG_ACPI
 	acpi_handle acpi_dev_handle;
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
@@ -565,7 +565,7 @@  static inline u32 tpm2_rc_value(u32 rc)
 }
 
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
-		  struct tpm_digest *digests);
+		  struct tpm_digest *digests, u16 *sizes);
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 		    struct tpm_digest *digests);
 int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 601d67c76c1e..51d2454c5329 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -181,11 +181,12 @@  struct tpm2_pcr_read_out {
  * @pcr_idx:	index of the PCR to read.
  * @count:	number of digests passed.
  * @digests:	list of pcr banks and buffers current PCR values are written to.
+ * @sizes:	list of digest sizes.
  *
  * Return: Same as with tpm_transmit_cmd.
  */
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
-		  struct tpm_digest *digests)
+		  struct tpm_digest *digests, u16 *sizes)
 {
 	int rc;
 	struct tpm_buf buf;
@@ -215,6 +216,9 @@  int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u32 count,
 		out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
 		memcpy(digests[0].digest, out->digest,
 		       be16_to_cpu(out->digest_size));
+
+		if (sizes)
+			sizes[0] = be16_to_cpu(out->digest_size);
 	}
 
 	tpm_buf_destroy(&buf);
@@ -245,7 +249,6 @@  int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 	struct tpm2_null_auth_area auth_area;
 	int rc;
 	int i;
-	int j;
 
 	if (count > ARRAY_SIZE(chip->active_banks))
 		return -EINVAL;
@@ -267,14 +270,9 @@  int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 	tpm_buf_append_u32(&buf, count);
 
 	for (i = 0; i < count; i++) {
-		for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) {
-			if (digests[i].alg_id != tpm2_hash_map[j].tpm_id)
-				continue;
-			tpm_buf_append_u16(&buf, digests[i].alg_id);
-			tpm_buf_append(&buf, (const unsigned char
-					      *)&digests[i].digest,
-			       hash_digest_size[tpm2_hash_map[j].crypto_id]);
-		}
+		tpm_buf_append_u16(&buf, digests[i].alg_id);
+		tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
+			       chip->active_banks[i].digest_size);
 	}
 
 	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
@@ -851,6 +849,26 @@  int tpm2_probe(struct tpm_chip *chip)
 }
 EXPORT_SYMBOL_GPL(tpm2_probe);
 
+static int tpm2_init_active_bank_info(struct tpm_chip *chip,
+				      struct tpm_active_bank_info *active_bank)
+{
+	struct tpm_digest digest = {.alg_id = active_bank->alg_id};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+		enum hash_algo crypto_algo = tpm2_hash_map[i].crypto_id;
+
+		if (active_bank->alg_id != tpm2_hash_map[i].tpm_id)
+			continue;
+
+		active_bank->digest_size = hash_digest_size[crypto_algo];
+		active_bank->crypto_id = crypto_algo;
+		return 0;
+	}
+
+	return tpm2_pcr_read(chip, 0, 1, &digest, &active_bank->digest_size);
+}
+
 struct tpm2_pcr_selection {
 	__be16  hash_alg;
 	u8  size_of_select;
@@ -905,7 +923,12 @@  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);
+		chip->active_banks[i].alg_id =
+			be16_to_cpu(pcr_selection.hash_alg);
+		rc =  tpm2_init_active_bank_info(chip, &chip->active_banks[i]);
+		if (rc)
+			break;
+
 		sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +
 			sizeof(pcr_selection.size_of_select) +
 			pcr_selection.size_of_select;
@@ -914,7 +937,7 @@  static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 
 out:
 	if (i < ARRAY_SIZE(chip->active_banks))
-		chip->active_banks[i] = TPM_ALG_ERROR;
+		chip->active_banks[i].alg_id = TPM_ALG_ERROR;
 
 	tpm_buf_destroy(&buf);
 
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index ac9fc47f4494..c8372ff582c3 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -46,6 +46,12 @@  struct tpm_digest {
 	u8 digest[SHA512_DIGEST_SIZE];
 } __packed;
 
+struct tpm_active_bank_info {
+	u16 alg_id;
+	u16 digest_size;
+	u16 crypto_id;
+};
+
 enum TPM_OPS_FLAGS {
 	TPM_OPS_AUTO_STARTUP = BIT(0),
 };