[PATCH] tpm: ibmvtpm: Call tpm2_sessions_init() to initialize session support

[James Bottomley]

On Mon, 2024-06-17 at 15:56 -0400, Stefan Berger wrote:
> 
> 
> On 6/17/24 15:42, James Bottomley wrote:
> > On Mon, 2024-06-17 at 15:34 -0400, Stefan Berger wrote:
> > > Fix the following type of error message caused by a missing call
> > > to
> > > tpm2_sessions_init() in the IBM vTPM driver:
> > > 
> > > [    2.987131] tpm tpm0: tpm2_load_context: failed with a TPM
> > > error
> > > 0x01C4
> > > [    2.987140] ima: Error Communicating to TPM chip, result: -14
> > > 
> > > Fixes: d2add27cf2b8 ("tpm: Add NULL primary creation")
> > > Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
> > > ---
> > >   drivers/char/tpm/tpm_ibmvtpm.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c
> > > b/drivers/char/tpm/tpm_ibmvtpm.c
> > > index d3989b257f42..1e5b107d1f3b 100644
> > > --- a/drivers/char/tpm/tpm_ibmvtpm.c
> > > +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> > > @@ -698,6 +698,10 @@ static int tpm_ibmvtpm_probe(struct vio_dev
> > > *vio_dev,
> > >                  rc = tpm2_get_cc_attrs_tbl(chip);
> > >                  if (rc)
> > >                          goto init_irq_cleanup;
> > > +
> > > +               rc = tpm2_sessions_init(chip);
> > > +               if (rc)
> > > +                       goto init_irq_cleanup;
> > 
> > This looks wrong: the whole thing is designed to occur in the
> > bootstrap
> > phase from tpm_chip_register() (which tpm_ibmvtpm.c definitely
> > calls),
> > so why isn't it happening?
> 
> Because flags = TPM_OPS_AUTO_STARTUP has not been set for this
> driver.
> 

In that case, wouldn't the fix be to move tpm_sessions_init() to
somewhere in tpm_chip_register() that would then be called by this
driver?  Having to special case it for every driver that doesn't set
this flag is going to be a huge pain.

I think the only reason it's down that far is that it should only be
called for TPM2 code so it was avoiding doing the check twice, so
something like this?

James

---

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 5da134f12c9a..4280cbb0f0b1 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -347,6 +347,12 @@ int tpm_auto_startup(struct tpm_chip *chip)
 {
 	int rc;
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		rc = tpm2_sessions_init(chip);
+		if (rc)
+			return rc;
+	}
+
 	if (!(chip->ops->flags & TPM_OPS_AUTO_STARTUP))
 		return 0;
 
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 1e856259219e..b4f85c8cdbb6 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -773,11 +773,6 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		rc = 0;
 	}
 
-	if (rc)
-		goto out;
-
-	rc = tpm2_sessions_init(chip);
-
 out:
 	/*
 	 * Infineon TPM in field upgrade mode will return no data for the number