linux-kernel - Re: [PATCH] tpm: ibmvtpm: Call tpm2_sessions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 17 Jun 2024 16:05:33 -0400
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: Stefan Berger <stefanb@...ux.ibm.com>, linux-integrity@...r.kernel.org, 
	linuxppc-dev@...ts.ozlabs.org, jarkko@...nel.org
Cc: linux-kernel@...r.kernel.org, mpe@...erman.id.au, 
	naveen.n.rao@...ux.ibm.com
Subject: Re: [PATCH] tpm: ibmvtpm: Call tpm2_sessions_init() to initialize
 session support

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@...ux.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