| Message ID | 20231208053914.216413-1-masahisa.kojima@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | efi_loader: fix iteration of FMP protocols | expand |
Hi Kojima-san On Fri, 8 Dec 2023 at 07:40, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: > > If one of the FMP protocols fails when calling GetImageInfo(), > populating the ESRT ends up with failure and other FMP protocols > are not added to the ESRT. We should still add all other FMP > protocols to the ESRT. > > With this commit, iteration of all FMP protocols continues > even though one of the FMP protocols fails. > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > Note that this patch addresses the following issue. > https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/3 [...] > > @@ -381,14 +381,14 @@ efi_status_t efi_esrt_populate(void) > */ > EFI_PRINT("ESRT erroneous FMP implementation\n"); > ret = EFI_INVALID_PARAMETER; > - goto out; > + continue; This doesn't look correct now. We set ret but we never exit to do something with it, instead the result is overwritten on the next iteration. I think we should only get rid of the assignment, unless I am missing something > } > > ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size, [...] Thanks /Ilias
On Fri, 15 Dec 2023 at 19:07, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Kojima-san > > On Fri, 8 Dec 2023 at 07:40, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: > > > > If one of the FMP protocols fails when calling GetImageInfo(), > > populating the ESRT ends up with failure and other FMP protocols > > are not added to the ESRT. We should still add all other FMP > > protocols to the ESRT. > > > > With this commit, iteration of all FMP protocols continues > > even though one of the FMP protocols fails. > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > > --- > > Note that this patch addresses the following issue. > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/3 > > [...] > > > > > @@ -381,14 +381,14 @@ efi_status_t efi_esrt_populate(void) > > */ > > EFI_PRINT("ESRT erroneous FMP implementation\n"); > > ret = EFI_INVALID_PARAMETER; > > - goto out; > > + continue; > > This doesn't look correct now. We set ret but we never exit to do > something with it, instead the result is overwritten on the next > iteration. I think we should only get rid of the assignment, unless I > am missing something Yes, we need to remove "ret = EFI_INVALID_PARAMETER;" line. Thanks, Masahisa Kojima > > > } > > > > ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size, > > [...] > > Thanks > /Ilias
diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c index 7f46d651e6..e5670c4f5e 100644 --- a/lib/efi_loader/efi_esrt.c +++ b/lib/efi_loader/efi_esrt.c @@ -365,7 +365,7 @@ efi_status_t efi_esrt_populate(void) if (ret != EFI_SUCCESS) { EFI_PRINT("ESRT Unable to find FMP handle (%u)\n", idx); - goto out; + continue; } fmp = handler->protocol_interface; @@ -381,14 +381,14 @@ efi_status_t efi_esrt_populate(void) */ EFI_PRINT("ESRT erroneous FMP implementation\n"); ret = EFI_INVALID_PARAMETER; - goto out; + continue; } ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size, (void **)&img_info); if (ret != EFI_SUCCESS) { EFI_PRINT("ESRT failed to allocate memory for image info\n"); - goto out; + continue; } /* @@ -406,7 +406,7 @@ efi_status_t efi_esrt_populate(void) if (ret != EFI_SUCCESS) { EFI_PRINT("ESRT failed to obtain image info from FMP\n"); efi_free_pool(img_info); - goto out; + continue; } num_entries += desc_count; @@ -437,7 +437,7 @@ efi_status_t efi_esrt_populate(void) if (ret != EFI_SUCCESS) { EFI_PRINT("ESRT unable to find FMP handle (%u)\n", idx); - break; + continue; } fmp = handler->protocol_interface;
If one of the FMP protocols fails when calling GetImageInfo(), populating the ESRT ends up with failure and other FMP protocols are not added to the ESRT. We should still add all other FMP protocols to the ESRT. With this commit, iteration of all FMP protocols continues even though one of the FMP protocols fails. Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- Note that this patch addresses the following issue. https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/3 lib/efi_loader/efi_esrt.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)