On Fri, May 06, 2022 at 02:00:41PM +0200, Stefan Mahnke-Hartmann wrote:
> > On Mon, Apr 25, 2022 at 04:03:33PM +0200, Jarkko Sakkinen wrote:
> > > On Mon, 2022-04-25 at 12:25 +0200, Mårten Lindahl wrote:
> > > > In commit 0aa698787aa2 ("tpm: Add Upgrade/Reduced mode support for
> > > > TPM2 modules") it was said that:
> > > >
> > > > "If the TPM is in Failure mode, it will successfully respond to both
> > > > tpm2_do_selftest() and tpm2_startup() calls. Although, will fail to
> > > > answer to tpm2_get_cc_attrs_tbl(). Use this fact to conclude that TPM
> > > > is in Failure mode."
> > > >
> > > > But a check was never added in the commit when calling
> > > > tpm2_get_cc_attrs_tbl() to conclude that the TPM is in Failure mode.
> > > > This commit corrects this by adding a check.
> > > >
> > > > Signed-off-by: Mårten Lindahl <marten.lindahl@xxxxxxxx>
> > >
> > > Reviewed-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
> > >
> > > I think this should actually have the fixes tag. It's a regression,
> > > right? I can add it.
> >
> > Hi Jarkko!
> >
> > Thank you. Yes it should have a fixes tag for the mentioned commit.
> >
> > Kind regards
> > Mårten
> >
> > >
> > > BR, Jarkko
>
> Hi Mårten,
>
> your patch introduces redundant code. The code block of the if below
> contains exactly the same code.
Hi!
The checks are for different cases. The TPM may sucessfully pass
startup and selftest but still be in failure mode. In this case only
some capabilities are required to work (Trusted Platform Module
Library Specification, Family "2.0", Level 00, Revision 01.59 –
November 2019 Part 3, page 348 and 349), but other capabilities may
fail. So in order for userspace to handle the TPM, this check is
needed.
The already existing check (below the out label) is specifically for
upgrade mode.
Yes the code blocks looks very similar, though the print is different.
>
> Additionally, your patch masks a bug (derefencing uninitialized
> memory). I will send a patch set shortly.
Masking bugs are never good. Please send a patch for this.
Kind regards
Mårten
>
> Regards
> Stefan
