Re: [PATCH 0/2] tpm: Stop retrying TPMs that have stopped working

[Breno Leitao <leitao@debian.org>]

On Thu, Nov 12, 2026 at 04:01:47AM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 12, 2026 at 03:57:29AM +0200, Jarkko Sakkinen wrote:
> > On Thu, Nov 12, 2026 at 12:46:57AM +0200, Jarkko Sakkinen wrote:
> > > On Thu, Nov 12, 2026 at 12:43:59AM +0200, Jarkko Sakkinen wrote:
> > > > $ tpm2sh return-code 256
> > > > TPM_RC_INITIALIZE
> > > > 
> > > > I.e. TPM has not been properly initialized by TPM2_Startup().
> > > > 
> > > > I.e. TPM is also saying that it is *disabled*.
> > > > 
> > > > So acting on this proven fact  makes most sense to me.
> > > 
> > > 
> > > Actually, should we even disable TPM if it returns this error code
> > > already at the level of tpm_transmit(). It is a guaranteed dead-end
> > > to any possible TPM operation.
> > > 
> > > I.e. while this fixes hwrng issue I think it might be addressing the
> > > issue at a wrong level.
> > 
> > I leave room for figuring out 1/2 but for the second issue I would just
> > do what the attached patch does and call it a day.
> > 
> > Br, Jarkko
> 
> > From 093124e51a26de8191e21ce41988532ea7e0bc8a Mon Sep 17 00:00:00 2001
> > From: Jarkko Sakkinen <jarkko@kernel.org>
> > Date: Thu, 12 Nov 2026 03:55:00 +0200
> > Subject: [PATCH] tpm: Disable TPM on TPM_RC_INITIALIZE
> > 
> > If the TPM has been spuriously disabled, the game is over. Let's reflect
> > this truth with TPM_CHIP_FLAG_DISABLE.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> >  drivers/char/tpm/tpm-interface.c | 5 +++++
> >  drivers/char/tpm/tpm2-cmd.c      | 2 ++
> >  2 files changed, 7 insertions(+)
> > 
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > index 470b2da3234e..1f1e22341155 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -256,6 +256,11 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz)
> >  		if (rc == TPM2_RC_TESTING && cc == TPM2_CC_SELF_TEST)
> >  			break;
> >  
> > +		if (rc == TPM_RC_INITIALIZE) {
> > +			chip->flag |= TPM_CHIP_FLAG_DISABLE;
> > +			break;
> > +		}
> > +
> 
> Sorry, should be TPM2_RC_INITIALIZE. 

Ack. Thanks for the suggestion, I will include it on my v2 then, but,
while testing, I needed to change a few things:

1) The check needs to go before the RETRY/TESTING break, otherwise an
   RC_INITIALIZE reply leaves the loop before reaching it.

2) It needs to skip TPM2_Startup:
          if (rc == TPM2_RC_INITIALIZE && cc != TPM2_CC_STARTUP) {

A TPM that is already started answers TPM2_Startup with
TPM_RC_INITIALIZE as well, so without the cc check a redundant
TPM2_Startup disables a healthy TPM. I hit it with swtpm's vTPM proxy,
which sends its own TPM2_Startup right after the one from
tpm2_auto_startup().

Let me respin a v2 with these changes I have been testing, and we can
continue on that thread.

Thanks for the review and suggestion,
--breno