On 07/05/2019 10:13 AM, Stefan Berger wrote:
On 7/3/19 11:32 PM, Nayna Jain wrote:
The nr_allocated_banks and allocated banks are initialized as part of
tpm_chip_register. Currently, this is done as part of auto startup
function. However, some drivers, like the ibm vtpm driver, do not run
auto startup during initialization. This results in uninitialized memory
issue and causes a kernel panic during boot.
This patch moves the pcr allocation outside the auto startup function
into tpm_chip_register. This ensures that allocated banks are
initialized
in any case.
Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms
with
PCR read")
Signed-off-by: Nayna Jain <nayna@xxxxxxxxxxxxx>
---
drivers/char/tpm/tpm-chip.c | 37 +++++++++++++++++++++++++++++++++++++
drivers/char/tpm/tpm.h | 1 +
drivers/char/tpm/tpm1-cmd.c | 12 ------------
drivers/char/tpm/tpm2-cmd.c | 6 +-----
4 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 8804c9e916fd..958508bb8379 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -550,6 +550,39 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
return hwrng_register(&chip->hwrng);
}
+/*
+ * tpm_pcr_allocation() - initializes the chip allocated banks for PCRs
+ */
+static int tpm_pcr_allocation(struct tpm_chip *chip)
+{
+ int rc = 0;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ rc = tpm2_get_pcr_allocation(chip);
+ if (rc)
+ goto out;
+ }
+
+ /* Initialize TPM 1.2 */
+ chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
+ GFP_KERNEL);
+ if (!chip->allocated_banks) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
+ chip->allocated_banks[0].digest_size =
hash_digest_size[HASH_ALGO_SHA1];
+ chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
+ chip->nr_allocated_banks = 1;
+
+ return 0;
+out:
+ if (rc < 0)
+ rc = -ENODEV;
The old code where you lifted this from said:
out:
if (rc > 0)
rc = -ENODEV;
return rc;
It would not overwrite -ENOMEM with -ENODEV but yours does.
I think the correct fix would be to use:
if (rc > 0)
rc = -ENODEV;
Yes. I think I misread it. Thanks Stefan. Will fix this..
+ return rc;
+}
+
/*
* tpm_chip_register() - create a character device for the TPM chip
* @chip: TPM chip to use.
@@ -573,6 +606,10 @@ int tpm_chip_register(struct tpm_chip *chip)
if (rc)
return rc;
Above this is tpm_chip_stop(chip) because (afaik) none of the
following function calls in tpm_chip_register() needed the TPM, but
now with tpm_pcr_allocation() you will need to send a command to the
TPM. So I would say you should move the tpm_chip_stop() into the error
branch visible above and also after the tpm_pcr_allocation().
+ rc = tpm_pcr_allocation(chip);
tpm_chip_stop(chip);
I am not sure of the purpose of tpm_stop_chip(), so I have left it as it
is. Jarkko, what do you think about the change ?
Thanks & Regards,
- Nayna
