[PATCH] Apply on top of: tpm: dynamically allocate active_banks array

[PATCH] Apply on top of: tpm: dynamically allocate active_banks array

[Roberto Sassu]

This patch checks if a PCR bank is active (at least one bit in the
pcr_select mask is set).
---
 drivers/char/tpm/tpm2-cmd.c | 44 +++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 533089cede07..eba8f8d00a09 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -854,15 +854,20 @@ struct tpm2_pcr_selection {
 
 static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 {
-	struct tpm2_pcr_selection pcr_selection;
+	struct tpm2_pcr_selection *pcr_selection;
 	struct tpm_buf buf;
 	void *marker;
 	void *end;
 	void *pcr_select_offset;
 	u32 sizeof_pcr_selection;
+	u32 nr_possible_banks;
+	u32 nr_active_banks = 0;
+	u16 *possible_banks;
+	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)
@@ -877,13 +882,12 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 	if (rc)
 		goto out;
 
-	chip->nr_active_banks = be32_to_cpup(
+	nr_possible_banks = be32_to_cpup(
 		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
 
-	chip->active_banks = kmalloc_array(chip->nr_active_banks,
-					   sizeof(*chip->active_banks),
-					   GFP_KERNEL);
-	if (!chip->active_banks) {
+	possible_banks = kmalloc_array(nr_possible_banks,
+				       sizeof(*possible_banks), GFP_KERNEL);
+	if (!possible_banks) {
 		rc = -ENOMEM;
 		goto out;
 	}
@@ -893,7 +897,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 < chip->nr_active_banks; 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) {
@@ -901,14 +905,30 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 			break;
 		}
 
-		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
-		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
-		sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +
-			sizeof(pcr_selection.size_of_select) +
-			pcr_selection.size_of_select;
+		pcr_selection = (struct tpm2_pcr_selection *)marker;
+		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)
+			possible_banks[nr_active_banks++] = hash_alg;
+
+		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_active_banks = nr_active_banks;
+	chip->active_banks = kmemdup(possible_banks,
+				     nr_active_banks * sizeof(*possible_banks),
+				     GFP_KERNEL);
+	if (!chip->active_banks)
+		rc = -ENOMEM;
+
+	kfree(possible_banks);
 out:
 	tpm_buf_destroy(&buf);
 
-- 
2.17.1

Re: [PATCH] Apply on top of: tpm: dynamically allocate active_banks array

[Nayna Jain]

On 11/09/2018 02:59 PM, Roberto Sassu wrote:
> This patch checks if a PCR bank is active (at least one bit in the
> pcr_select mask is set).
> ---
>   drivers/char/tpm/tpm2-cmd.c | 44 +++++++++++++++++++++++++++----------
>   1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 533089cede07..eba8f8d00a09 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -854,15 +854,20 @@ struct tpm2_pcr_selection {
>   
>   static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>   {
> -	struct tpm2_pcr_selection pcr_selection;
> +	struct tpm2_pcr_selection *pcr_selection;
>   	struct tpm_buf buf;
>   	void *marker;
>   	void *end;
>   	void *pcr_select_offset;
>   	u32 sizeof_pcr_selection;
> +	u32 nr_possible_banks;
> +	u32 nr_active_banks = 0;

The TCG Spec uses the term "allocated" and not "active".  For 
consistency, I think we can also use the term "allocated". For example - 
nr_allocated_banks and *allocated_banks both here and in struct tpm_chip.

Sorry, it took me sometime to be clear on this else would have mentioned 
previously.

Thanks & Regards,
     - Nayna

Re: [PATCH] Apply on top of: tpm: dynamically allocate active_banks array

[Roberto Sassu]

On 11/12/2018 7:09 PM, Nayna Jain wrote:
> 
> 
> On 11/09/2018 02:59 PM, Roberto Sassu wrote:
>> This patch checks if a PCR bank is active (at least one bit in the
>> pcr_select mask is set).
>> ---
>>   drivers/char/tpm/tpm2-cmd.c | 44 +++++++++++++++++++++++++++----------
>>   1 file changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 533089cede07..eba8f8d00a09 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -854,15 +854,20 @@ struct tpm2_pcr_selection {
>>   static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>>   {
>> -    struct tpm2_pcr_selection pcr_selection;
>> +    struct tpm2_pcr_selection *pcr_selection;
>>       struct tpm_buf buf;
>>       void *marker;
>>       void *end;
>>       void *pcr_select_offset;
>>       u32 sizeof_pcr_selection;
>> +    u32 nr_possible_banks;
>> +    u32 nr_active_banks = 0;
> 
> The TCG Spec uses the term "allocated" and not "active".  For 
> consistency, I think we can also use the term "allocated". For example - 
> nr_allocated_banks and *allocated_banks both here and in struct tpm_chip.
> 
> Sorry, it took me sometime to be clear on this else would have mentioned 
> previously.

Thanks. Jarkko?

Roberto


> Thanks & Regards,
>      - Nayna
> 
> 
> 

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI

Re: [PATCH] Apply on top of: tpm: dynamically allocate active_banks array

[Jarkko Sakkinen]

On Tue, Nov 13, 2018 at 10:49:31AM +0100, Roberto Sassu wrote:
> On 11/12/2018 7:09 PM, Nayna Jain wrote:
> > 
> > 
> > On 11/09/2018 02:59 PM, Roberto Sassu wrote:
> > > This patch checks if a PCR bank is active (at least one bit in the
> > > pcr_select mask is set).
> > > ---
> > >   drivers/char/tpm/tpm2-cmd.c | 44 +++++++++++++++++++++++++++----------
> > >   1 file changed, 32 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > > index 533089cede07..eba8f8d00a09 100644
> > > --- a/drivers/char/tpm/tpm2-cmd.c
> > > +++ b/drivers/char/tpm/tpm2-cmd.c
> > > @@ -854,15 +854,20 @@ struct tpm2_pcr_selection {
> > >   static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> > >   {
> > > -    struct tpm2_pcr_selection pcr_selection;
> > > +    struct tpm2_pcr_selection *pcr_selection;
> > >       struct tpm_buf buf;
> > >       void *marker;
> > >       void *end;
> > >       void *pcr_select_offset;
> > >       u32 sizeof_pcr_selection;
> > > +    u32 nr_possible_banks;
> > > +    u32 nr_active_banks = 0;
> > 
> > The TCG Spec uses the term "allocated" and not "active".  For
> > consistency, I think we can also use the term "allocated". For example -
> > nr_allocated_banks and *allocated_banks both here and in struct
> > tpm_chip.
> > 
> > Sorry, it took me sometime to be clear on this else would have mentioned
> > previously.
> 
> Thanks. Jarkko?
> 
> Roberto

Agree with Nayna.

/Jarkko