| Message ID | 657a869c04e9b09e3bd2e6fd74ff94320b7fbe9b.1638191161.git.michal.simek@xilinx.com |
|---|---|
| State | Superseded |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: Do not show error message if TPM is not present | expand |
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. > > -- > 2.33.1 > Thanks /Ilias
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; + } switch (handle->image_type) { case IMAGE_SUBSYSTEM_EFI_APPLICATION: M
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
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
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(-)