Re: [PATCH] efi_loader: Do not show error message if TPM is not present

From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
To: Michal Simek <michal.simek@xilinx.com>
Cc: u-boot@lists.denx.de, git@xilinx.com,
	Alexander Graf <agraf@csgraf.de>,
	 Heinrich Schuchardt <xypron.glpk@gmx.de>
Subject: Re: [PATCH] efi_loader: Do not show error message if TPM is not present
Date: Mon, 29 Nov 2021 15:52:31 +0200	[thread overview]
Message-ID: <CAC_iWjK+sGxtib4ahJrrQRCmR3GyBKm7qVOqWf0NCSwBPs6EJg@mail.gmail.com> (raw)
In-Reply-To: <e735e51b-b17c-3adf-bc83-c280102b4061@xilinx.com>

Hi Michal

On Mon, 29 Nov 2021 at 15:44, Michal Simek <michal.simek@xilinx.com> wrote:
>
>
>
> On 11/29/21 14:30, Ilias Apalodimas wrote:
> > Hi Michal,
> >
> > On Mon, 29 Nov 2021 at 15:06, Michal Simek <michal.simek@xilinx.com> wrote:
> >>
> >> For systems which have TPM support enabled but actual device is missing
> >> there is no reason to show a message that measurement failed.
> >> That's why properly check error code which is returned.
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >>
> >>   lib/efi_loader/efi_image_loader.c | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> >> index eb95580538cc..c6a254dc25dd 100644
> >> --- a/lib/efi_loader/efi_image_loader.c
> >> +++ b/lib/efi_loader/efi_image_loader.c
> >> @@ -934,8 +934,9 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
> >>
> >>   #if CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL)
> >>          /* Measure an PE/COFF image */
> >> -       if (tcg2_measure_pe_image(efi, efi_size, handle,
> >> -                                 loaded_image_info))
> >> +       ret = tcg2_measure_pe_image(efi, efi_size, handle,
> >> +                                   loaded_image_info);
> >> +       if (ret && ret != EFI_NOT_FOUND)
> >>                  log_err("PE image measurement failed\n");
> >>   #endif
> >
> > Indeed that's needed.  Looking at it again though maybe it's better to
> > add an identical check in tcg2_measure_pe_image() and return
> > EFI_SUCCESS if platform_get_tpm2_device() returned EFI_NOT_FOUND.  The
> > reason is that other parts of the code return EFI_NOT_FOUND in that
> > function (e.g efi_search_protocol).  So we need to make sure we report
> > the error in that case.
>
> just like this?
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 8c1f22e3377b..db6e7488b7fb 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -887,8 +887,10 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64
> efi_size,
>          struct efi_handler *handler;
>
>          ret = platform_get_tpm2_device(&dev);
> -       if (ret != EFI_SUCCESS)
> +       if (ret != EFI_SUCCESS) {
> +               ret = EFI_SUCCESS;
>                  return ret;
> +       }
>

Yea I don't expect platform_get_tpm2_device() to change.  Can you also
add a comment on why we do that for  future readers?

Cheers
/Ilias
>          switch (handle->image_type) {
>          case IMAGE_SUBSYSTEM_EFI_APPLICATION:
>
>
> M