| Message ID | 20181109092925.2908-1-roberto.sassu@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Apply on top of: tpm: dynamically allocate active_banks array | expand |
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
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 > > >
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
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);