Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Michael Chang]

On Thu, Feb 16, 2023 at 01:02:00PM -0500, Stefan Berger wrote:
> 
> 
> On 10/7/22 01:37, Michael Chang via Grub-devel wrote:
> > This helps to prevent out of memory error when reading large files via 
> > disabling
> > tpm device as verifier has to read all content into memory in one chunk to
> > measure the hash and extend to tpm.
> 
> For ibmvtpm driver support this change here would be need. Can you merge it 
> into your patch once the ibmvtpm driver is in the repo?
> 
> diff --git a/grub-core/commands/ieee1275/ibmvtpm.c 
> b/grub-core/commands/ieee1275/ibmvtpm.c
> index 239942d27..e01759c17 100644
> --- a/grub-core/commands/ieee1275/ibmvtpm.c
> +++ b/grub-core/commands/ieee1275/ibmvtpm.c
> @@ -135,16 +135,6 @@ grub_err_t
>  grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,
>                   const char *description)
>  {
> -  /*
> -   * Call tpm_init() 'late' rather than from GRUB_MOD_INIT() so that device 
> nodes
> -   * can be found.
> -   */
> -  grub_err_t err = tpm_init ();
> -
> -  /* Absence of a TPM isn't a failure. */
> -  if (err != GRUB_ERR_NONE)
> -    return GRUB_ERR_NONE;
> -
>    grub_dprintf ("tpm", "log_event, pcr = %d, size = 0x%" PRIxGRUB_SIZE ", 
> %s\n",
>                 pcr, size, description);
> 
> @@ -153,3 +143,13 @@ grub_tpm_measure (unsigned char *buf, grub_size_t size, 
> grub_uint8_t pcr,
> 
>    return GRUB_ERR_NONE;
>  }
> +
> +int
> +grub_tpm_present (void)
> +{
> +  /*
> +   * Call tpm_init() 'late' rather than from GRUB_MOD_INIT() so that device 
> nodes
> +   * can be found.
> +   */
> +  return tpm_init() == GRUB_ERR_NONE;
> +}
> 
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> 
> > diff --git a/grub-core/commands/tpm.c b/grub-core/commands/tpm.c
> > index 2052c36ea..cb8ed6b94 100644
> > --- a/grub-core/commands/tpm.c
> > +++ b/grub-core/commands/tpm.c
> > @@ -86,10 +86,14 @@ struct grub_file_verifier grub_tpm_verifier = {
> >   GRUB_MOD_INIT (tpm)
> >   {
> > +  if (!grub_tpm_present())
> > +    return;
> 
> Even though this now calls grub_tpm_present() from GRUB_MOD_INIT() and I have 
> this comment in the code
> above, it does seem to call it late enough in the initialization sequence so 
> that whatever discovered
> 'device nodes' before your GRUB_MOD_INIT() is called, enables the ibmvtpm 
> driver to see the device nodes.

Thanks to the clarification. I think it is worth to keep this comment in
the patched hunk so the reader won't get confused by the comment
followed in grub_tpm_present.

> 
> Per my tests powerpc grub now measures and logs for PCR 8 and 9 correctly and 
> also extends PCRs 8 & 9.
> I hope that nothing will change this initialization order because there seems 
> to be little control over it.

I will merge you change and submit a new version.

Thanks.
Michael

> 
>    Stefan
> 
> >     grub_verifier_register (&grub_tpm_verifier);
> >   }
> >   GRUB_MOD_FINI (tpm)
> >   {
> > +  if (!grub_tpm_present())
> > +    return;
> >     grub_verifier_unregister (&grub_tpm_verifier);
> >   }
> > diff --git a/include/grub/tpm.h b/include/grub/tpm.h
> > index 5c285cbc5..c19fcbd0a 100644
> > --- a/include/grub/tpm.h
> > +++ b/include/grub/tpm.h
> > @@ -36,4 +36,5 @@
> >   grub_err_t grub_tpm_measure (unsigned char *buf, grub_size_t size,
> >                          grub_uint8_t pcr, const char *description);
> > +int grub_tpm_present (void);
> >   #endif