| Message ID | 20231218095742.376839-1-masahisa.kojima@linaro.org |
|---|---|
| State | Accepted |
| Commit | 5a0970ee8cf4391835d21c59e44a65d54d42377c |
| Headers | show |
| Series | [v2] efi_loader: fix iteration of FMP protocols | expand |
On Mon, Dec 18, 2023 at 06:57:41PM +0900, Masahisa Kojima 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 > > changes in v2 > - add error handling when num_entries is 0 after calling fmp->get_image_info() > > lib/efi_loader/efi_esrt.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c > index 7f46d651e6..1f629a6023 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; > > @@ -380,15 +380,14 @@ efi_status_t efi_esrt_populate(void) > * fmp->get_image_info to return BUFFER_TO_SMALL. > */ > 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 +405,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; > @@ -414,6 +413,13 @@ efi_status_t efi_esrt_populate(void) > efi_free_pool(img_info); > } > > + /* error occurs in fmp->get_image_info() if num_entries is 0 here */ > + if (!num_entries) { > + EFI_PRINT("Error occurs, num_entries should not be 0\n"); > + ret = EFI_INVALID_PARAMETER; > + goto out; > + } > + > EFI_PRINT("ESRT create table with %u entries\n", num_entries); > /* > * Allocate an ESRT with the sufficient number of entries to accommodate > @@ -437,7 +443,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; > > -- > 2.34.1 > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> For the SynQuacer Tested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c index 7f46d651e6..1f629a6023 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; @@ -380,15 +380,14 @@ efi_status_t efi_esrt_populate(void) * fmp->get_image_info to return BUFFER_TO_SMALL. */ 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 +405,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; @@ -414,6 +413,13 @@ efi_status_t efi_esrt_populate(void) efi_free_pool(img_info); } + /* error occurs in fmp->get_image_info() if num_entries is 0 here */ + if (!num_entries) { + EFI_PRINT("Error occurs, num_entries should not be 0\n"); + ret = EFI_INVALID_PARAMETER; + goto out; + } + EFI_PRINT("ESRT create table with %u entries\n", num_entries); /* * Allocate an ESRT with the sufficient number of entries to accommodate @@ -437,7 +443,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 changes in v2 - add error handling when num_entries is 0 after calling fmp->get_image_info() lib/efi_loader/efi_esrt.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)