Re: [PATCH v7 5/5] tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()
On Mon, Jan 21, 2019 at 02:50:49PM +0100, Roberto Sassu wrote: > On 1/21/2019 1:37 PM, Jarkko Sakkinen wrote: > > 3. The would be nothing wrong exposing struct tpm_chip in > > include/linux/tpm.h. I would be totally fine with that. > > Should I do it in a separate patch (before 5/5)? Yes. > Is it fine to call tpm_default_chip() only in pcrlock() for trusted > keys? I think you should get the reference in init_trusted() (similar pattern as in IMA). /Jarkko
Re: [PATCH v7 5/5] tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()
On 1/21/2019 1:37 PM, Jarkko Sakkinen wrote: 3. The would be nothing wrong exposing struct tpm_chip in include/linux/tpm.h. I would be totally fine with that. Should I do it in a separate patch (before 5/5)? Is it fine to call tpm_default_chip() only in pcrlock() for trusted keys? Thanks Roberto /Jarkko -- HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Bo PENG, Jian LI, Yanli SHI
Re: [PATCH v7 5/5] tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()
On Mon, Jan 21, 2019 at 10:58:22AM +0100, Roberto Sassu wrote: > 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. 1. It is just plain wrong if the basic extend function invents its own dummy hashes when it doesn't get one. In the end, this is what matters to me, and I'm not going to accept anything that tries to do that. That is something that caller should prepare. You even left undocumented this very awkward and unguessable behaviour. I think redundancy is better than doing guesswork what a function might possibly do other than its basic task, which is constructing the extend command. 2. The driver should return -EINVAL, if it doesn't get all the hashes. 3. The would be nothing wrong exposing struct tpm_chip in include/linux/tpm.h. I would be totally fine with that. /Jarkko
Re: [PATCH v7 5/5] tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()
On Mon, Jan 21, 2019 at 09:11:21AM +0100, Roberto Sassu wrote:
> On 1/18/2019 4:12 PM, Jarkko Sakkinen wrote:
> > On Thu, Jan 17, 2019 at 08:59:00AM +0100, Roberto Sassu wrote:
> > > On 12/20/2018 4:21 PM, Jarkko Sakkinen wrote:
> > > > On Thu, Dec 13, 2018 at 11:29:45AM +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.
> > > > >
> > > > > The new tpm_extend digest 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
> > > > > ---
> > > > >drivers/char/tpm/tpm-interface.c | 24 +---
> > > > >drivers/char/tpm/tpm.h | 5 +++--
> > > > >drivers/char/tpm/tpm1-cmd.c| 13 ---
> > > > >drivers/char/tpm/tpm2-cmd.c| 35
> > > > > +-
> > > > >include/linux/tpm.h| 13 ---
> > > > >security/integrity/ima/ima_queue.c | 5 -
> > > > >security/keys/trusted.c| 5 -
> > > > >7 files changed, 62 insertions(+), 38 deletions(-)
> > > > >
> > > > > diff --git a/drivers/char/tpm/tpm-interface.c
> > > > > b/drivers/char/tpm/tpm-interface.c
> > > > > index eb7c79ca8a94..911fea19e408 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 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_extend_digest structures
> > > > > + * @digests: array of tpm_extend_digest structures used to extend
> > > > > PCRs
> > > > > *
> > > > > * 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_extend_digest *digests)
> > > >
> > > > Remove const. Document how @digests is used like the special meaning
> > > > of the first index. I faintly remember asking this last time.
> > > >
> > > > >{
> > > > > 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, digests);
> > > > > tpm_put_ops(chip);
> > > > > return rc;
> > > > > }
> > > > > - rc = tpm1_pcr_extend(chip, pcr_idx, hash,
> > > > > + rc = tpm1_pcr_extend(chip, pcr_idx, count, digests,
> > > > >"attempting extend a PCR value");
> > > >
> > > > The validation is missing that the provided array has only one element
> > > > and the algorithm is SHA1. Could be done also inside tpm1_pcr_extend()
> > > > but what you are doing to that function does not make any sense so
> > > > better to do it here.
> > > >
> > > > > tpm_put_ops(chip);
> > > > > return
Re: [PATCH v7 5/5] tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()
On 1/18/2019 4:12 PM, Jarkko Sakkinen wrote:
On Thu, Jan 17, 2019 at 08:59:00AM +0100, Roberto Sassu wrote:
On 12/20/2018 4:21 PM, Jarkko Sakkinen wrote:
On Thu, Dec 13, 2018 at 11:29:45AM +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.
The new tpm_extend digest 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
---
drivers/char/tpm/tpm-interface.c | 24 +---
drivers/char/tpm/tpm.h | 5 +++--
drivers/char/tpm/tpm1-cmd.c| 13 ---
drivers/char/tpm/tpm2-cmd.c| 35 +-
include/linux/tpm.h| 13 ---
security/integrity/ima/ima_queue.c | 5 -
security/keys/trusted.c| 5 -
7 files changed, 62 insertions(+), 38 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index eb7c79ca8a94..911fea19e408 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 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_extend_digest structures
+ * @digests: array of tpm_extend_digest structures used to extend PCRs
*
* 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_extend_digest *digests)
Remove const. Document how @digests is used like the special meaning
of the first index. I faintly remember asking this last time.
{
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, digests);
tpm_put_ops(chip);
return rc;
}
- rc = tpm1_pcr_extend(chip, pcr_idx, hash,
+ rc = tpm1_pcr_extend(chip, pcr_idx, count, digests,
"attempting extend a PCR value");
The validation is missing that the provided array has only one element
and the algorithm is SHA1. Could be done also inside tpm1_pcr_extend()
but what you are doing to that function does not make any sense so
better to do it here.
tpm_put_ops(chip);
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 64d93d26087f..6b446504d2fe 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -504,7 +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,
+int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
+ const struct tpm_extend_digest *digests,
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,
@@ -551,7 +552,7 @@ int tpm2_get_timeouts(struct tpm_chip *chip);
int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digest, u16 *digest_size_ptr);
int
Re: [PATCH v7 5/5] tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()
On 1/18/2019 4:12 PM, Jarkko Sakkinen wrote:
On Thu, Jan 17, 2019 at 08:59:00AM +0100, Roberto Sassu wrote:
On 12/20/2018 4:21 PM, Jarkko Sakkinen wrote:
On Thu, Dec 13, 2018 at 11:29:45AM +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.
The new tpm_extend digest 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
---
drivers/char/tpm/tpm-interface.c | 24 +---
drivers/char/tpm/tpm.h | 5 +++--
drivers/char/tpm/tpm1-cmd.c| 13 ---
drivers/char/tpm/tpm2-cmd.c| 35 +-
include/linux/tpm.h| 13 ---
security/integrity/ima/ima_queue.c | 5 -
security/keys/trusted.c| 5 -
7 files changed, 62 insertions(+), 38 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index eb7c79ca8a94..911fea19e408 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 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_extend_digest structures
+ * @digests: array of tpm_extend_digest structures used to extend PCRs
*
* 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_extend_digest *digests)
Remove const. Document how @digests is used like the special meaning
of the first index. I faintly remember asking this last time.
{
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, digests);
tpm_put_ops(chip);
return rc;
}
- rc = tpm1_pcr_extend(chip, pcr_idx, hash,
+ rc = tpm1_pcr_extend(chip, pcr_idx, count, digests,
"attempting extend a PCR value");
The validation is missing that the provided array has only one element
and the algorithm is SHA1. Could be done also inside tpm1_pcr_extend()
but what you are doing to that function does not make any sense so
better to do it here.
tpm_put_ops(chip);
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 64d93d26087f..6b446504d2fe 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -504,7 +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,
+int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
+ const struct tpm_extend_digest *digests,
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,
@@ -551,7 +552,7 @@ int tpm2_get_timeouts(struct tpm_chip *chip);
int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digest, u16 *digest_size_ptr);
int
Re: [PATCH v7 5/5] tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()
On Thu, Jan 17, 2019 at 08:59:00AM +0100, Roberto Sassu wrote:
> On 12/20/2018 4:21 PM, Jarkko Sakkinen wrote:
> > On Thu, Dec 13, 2018 at 11:29:45AM +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.
> > >
> > > The new tpm_extend digest 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
> > > ---
> > > drivers/char/tpm/tpm-interface.c | 24 +---
> > > drivers/char/tpm/tpm.h | 5 +++--
> > > drivers/char/tpm/tpm1-cmd.c| 13 ---
> > > drivers/char/tpm/tpm2-cmd.c| 35 +-
> > > include/linux/tpm.h| 13 ---
> > > security/integrity/ima/ima_queue.c | 5 -
> > > security/keys/trusted.c| 5 -
> > > 7 files changed, 62 insertions(+), 38 deletions(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm-interface.c
> > > b/drivers/char/tpm/tpm-interface.c
> > > index eb7c79ca8a94..911fea19e408 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 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_extend_digest structures
> > > + * @digests: array of tpm_extend_digest structures used to extend
> > > PCRs
> > >*
> > >* 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_extend_digest *digests)
> >
> > Remove const. Document how @digests is used like the special meaning
> > of the first index. I faintly remember asking this last time.
> >
> > > {
> > > 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, digests);
> > > tpm_put_ops(chip);
> > > return rc;
> > > }
> > > - rc = tpm1_pcr_extend(chip, pcr_idx, hash,
> > > + rc = tpm1_pcr_extend(chip, pcr_idx, count, digests,
> > >"attempting extend a PCR value");
> >
> > The validation is missing that the provided array has only one element
> > and the algorithm is SHA1. Could be done also inside tpm1_pcr_extend()
> > but what you are doing to that function does not make any sense so
> > better to do it here.
> >
> > > tpm_put_ops(chip);
> > > return rc;
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index 64d93d26087f..6b446504d2fe 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -504,7 +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,
> > > +int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>
Re: [PATCH v7 5/5] tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()
On 12/20/2018 4:21 PM, Jarkko Sakkinen wrote:
On Thu, Dec 13, 2018 at 11:29:45AM +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.
The new tpm_extend digest 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
---
drivers/char/tpm/tpm-interface.c | 24 +---
drivers/char/tpm/tpm.h | 5 +++--
drivers/char/tpm/tpm1-cmd.c| 13 ---
drivers/char/tpm/tpm2-cmd.c| 35 +-
include/linux/tpm.h| 13 ---
security/integrity/ima/ima_queue.c | 5 -
security/keys/trusted.c| 5 -
7 files changed, 62 insertions(+), 38 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index eb7c79ca8a94..911fea19e408 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 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_extend_digest structures
+ * @digests: array of tpm_extend_digest structures used to extend PCRs
*
* 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_extend_digest *digests)
Remove const. Document how @digests is used like the special meaning
of the first index. I faintly remember asking this last time.
{
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, digests);
tpm_put_ops(chip);
return rc;
}
- rc = tpm1_pcr_extend(chip, pcr_idx, hash,
+ rc = tpm1_pcr_extend(chip, pcr_idx, count, digests,
"attempting extend a PCR value");
The validation is missing that the provided array has only one element
and the algorithm is SHA1. Could be done also inside tpm1_pcr_extend()
but what you are doing to that function does not make any sense so
better to do it here.
tpm_put_ops(chip);
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 64d93d26087f..6b446504d2fe 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -504,7 +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,
+int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
+ const struct tpm_extend_digest *digests,
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,
@@ -551,7 +552,7 @@ int tpm2_get_timeouts(struct tpm_chip *chip);
int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digest, u16 *digest_size_ptr);
int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
- struct tpm_digest *digests);
+ const struct
Re: [PATCH v7 5/5] tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()
On Thu, Dec 13, 2018 at 11:29:45AM +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.
>
> The new tpm_extend digest 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
> ---
> drivers/char/tpm/tpm-interface.c | 24 +---
> drivers/char/tpm/tpm.h | 5 +++--
> drivers/char/tpm/tpm1-cmd.c| 13 ---
> drivers/char/tpm/tpm2-cmd.c| 35 +-
> include/linux/tpm.h| 13 ---
> security/integrity/ima/ima_queue.c | 5 -
> security/keys/trusted.c| 5 -
> 7 files changed, 62 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c
> b/drivers/char/tpm/tpm-interface.c
> index eb7c79ca8a94..911fea19e408 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 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_extend_digest structures
> + * @digests: array of tpm_extend_digest structures used to extend PCRs
> *
> * 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_extend_digest *digests)
Remove const. Document how @digests is used like the special meaning
of the first index. I faintly remember asking this last time.
> {
> 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, digests);
> tpm_put_ops(chip);
> return rc;
> }
>
> - rc = tpm1_pcr_extend(chip, pcr_idx, hash,
> + rc = tpm1_pcr_extend(chip, pcr_idx, count, digests,
>"attempting extend a PCR value");
The validation is missing that the provided array has only one element
and the algorithm is SHA1. Could be done also inside tpm1_pcr_extend()
but what you are doing to that function does not make any sense so
better to do it here.
> tpm_put_ops(chip);
> return rc;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 64d93d26087f..6b446504d2fe 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -504,7 +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,
> +int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> + const struct tpm_extend_digest *digests,
> 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,
> @@ -551,7 +552,7 @@ int tpm2_get_timeouts(struct tpm_chip *chip);
> int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
> struct tpm_digest *digest, u16 *digest_size_ptr);
> int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> - struct
[PATCH v7 5/5] tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()
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_extend digest 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
---
drivers/char/tpm/tpm-interface.c | 24 +---
drivers/char/tpm/tpm.h | 5 +++--
drivers/char/tpm/tpm1-cmd.c| 13 ---
drivers/char/tpm/tpm2-cmd.c| 35 +-
include/linux/tpm.h| 13 ---
security/integrity/ima/ima_queue.c | 5 -
security/keys/trusted.c| 5 -
7 files changed, 62 insertions(+), 38 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index eb7c79ca8a94..911fea19e408 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 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_extend_digest structures
+ * @digests: array of tpm_extend_digest structures used to extend PCRs
*
* 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_extend_digest *digests)
{
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, digests);
tpm_put_ops(chip);
return rc;
}
- rc = tpm1_pcr_extend(chip, pcr_idx, hash,
+ rc = tpm1_pcr_extend(chip, pcr_idx, count, digests,
"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 64d93d26087f..6b446504d2fe 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -504,7 +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,
+int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
+ const struct tpm_extend_digest *digests,
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,
@@ -551,7 +552,7 @@ int tpm2_get_timeouts(struct tpm_chip *chip);
int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digest, u16 *digest_size_ptr);
int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
- struct tpm_digest *digests);
+ const struct tpm_extend_digest *digests);
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..04ee10284b8c 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -449,12 +449,20 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
}
#define TPM_ORD_PCR_EXTEND 20
-int
