Re: [PATCH v3] tpm: tpm_ibm_vtpm: Fix unallocated banks

2019-07-16 Thread Michal Suchánek
On Fri, 12 Jul 2019 00:13:57 +0300
Jarkko Sakkinen  wrote:

> On Thu, Jul 11, 2019 at 11:28:24PM +0300, Jarkko Sakkinen wrote:
> > On Thu, Jul 11, 2019 at 12:13:35PM -0400, 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")
> > > Reported-by: Michal Suchanek 
> > > Signed-off-by: Nayna Jain 
> > > Reviewed-by: Mimi Zohar 
> > > Tested-by: Sachin Sant 
> > > Tested-by: Michal Suchánek   
> > 
> > Reviewed-by: Jarkko Sakkinen   
> 
> Thanks a lot! It is applied now.

Fixes the issue for me.

Thanks

Michal


Re: [PATCH v3] tpm: tpm_ibm_vtpm: Fix unallocated banks

2019-07-11 Thread Jarkko Sakkinen
On Thu, Jul 11, 2019 at 11:28:24PM +0300, Jarkko Sakkinen wrote:
> On Thu, Jul 11, 2019 at 12:13:35PM -0400, 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")
> > Reported-by: Michal Suchanek 
> > Signed-off-by: Nayna Jain 
> > Reviewed-by: Mimi Zohar 
> > Tested-by: Sachin Sant 
> > Tested-by: Michal Suchánek 
> 
> Reviewed-by: Jarkko Sakkinen 

Thanks a lot! It is applied now.

/Jarkko


Re: [PATCH v3] tpm: tpm_ibm_vtpm: Fix unallocated banks

2019-07-11 Thread Jarkko Sakkinen
On Thu, Jul 11, 2019 at 12:13:35PM -0400, 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")
> Reported-by: Michal Suchanek 
> Signed-off-by: Nayna Jain 
> Reviewed-by: Mimi Zohar 
> Tested-by: Sachin Sant 
> Tested-by: Michal Suchánek 

Reviewed-by: Jarkko Sakkinen 

/Jarkko


[PATCH v3] tpm: tpm_ibm_vtpm: Fix unallocated banks

2019-07-11 Thread Nayna Jain
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")
Reported-by: Michal Suchanek 
Signed-off-by: Nayna Jain 
Reviewed-by: Mimi Zohar 
Tested-by: Sachin Sant 
Tested-by: Michal Suchánek 
---
Changelog:
v3:
* Includes Stefan's feedback correctly:
  * Fixed handling of rc > 0 error
* Includes Jarkko's feedback related to comment and the function.

v2:
* Includes Jarkko's feedbacks
  * fixes the function name to tpm_get_pcr_allocation()
  * adds new function tpm1_get_pcr_allocation()
  * updates patch summary line
  * fixes alignment
  * adds Reported-by: Michal Suchanek 
* Includes Stefan's feedbacks
  * Fixes overwriting of return code
  * Fixes misplacing of tpm_chip_stop()
* Adds Reviewed-by, Tested-by

 drivers/char/tpm/tpm-chip.c | 20 
 drivers/char/tpm/tpm.h  |  2 ++
 drivers/char/tpm/tpm1-cmd.c | 36 
 drivers/char/tpm/tpm2-cmd.c |  6 +-
 4 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 8804c9e916fd..5a0396d6560d 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -550,6 +550,20 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
return hwrng_register(>hwrng);
 }
 
+static int tpm_get_pcr_allocation(struct tpm_chip *chip)
+{
+   int rc;
+
+   rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ?
+tpm2_get_pcr_allocation(chip) :
+tpm1_get_pcr_allocation(chip);
+
+   if (rc > 0)
+   return -ENODEV;
+
+   return rc;
+}
+
 /*
  * tpm_chip_register() - create a character device for the TPM chip
  * @chip: TPM chip to use.
@@ -569,6 +583,12 @@ int tpm_chip_register(struct tpm_chip *chip)
if (rc)
return rc;
rc = tpm_auto_startup(chip);
+   if (rc) {
+   tpm_chip_stop(chip);
+   return rc;
+   }
+
+   rc = tpm_get_pcr_allocation(chip);
tpm_chip_stop(chip);
if (rc)
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 2cce072f25b5..d571df3694c3 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -399,6 +399,7 @@ 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,
const char *desc, size_t min_cap_length);
 int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+int tpm1_get_pcr_allocation(struct tpm_chip *chip);
 unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
 int tpm_pm_suspend(struct device *dev);
 int tpm_pm_resume(struct device *dev);
@@ -454,6 +455,7 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
u32 *value, const char *desc);
 
+ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
 int tpm2_auto_startup(struct tpm_chip *chip);
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 85dcf2654d11..260a3917f0fe 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -696,18 +696,6 @@ int tpm1_auto_startup(struct tpm_chip *chip)
goto out;
}
 
-   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 rc;
 out:
if (rc > 0)
@@ -776,3 +764,27 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 
tpm_suspend_pcr)
return rc;
 }
 
+/**
+ * tpm1_get_pcr_allocation() - initialize the allocated bank
+ * @chip: TPM chip to use.
+ *
+ * The function initializes the SHA1 allocated bank to extend PCR
+ *
+ * Return:
+ * * 0 on success,
+ * * < 0 on error.
+ */
+int tpm1_get_pcr_allocation(struct tpm_chip *chip)
+{
+   chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
+   GFP_KERNEL);
+   if (!chip->allocated_banks)
+