[PATCH 1/1] efi_loader: CloseProtocol in efi_fmp_find

[Ilias Apalodimas]

Hi Heinrich

On Fri, 7 Oct 2022 at 17:06, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> The CloseProtocol() boot service requires a handle as first argument.
> Passing the protocol interface is incorrect.
>
> CloseProtocol() only has an effect if called with a non-zero value for
> agent_handle. HandleProtocol() uses an opaque agent_handle when invoking
> OpenProtocol() (currently NULL). Therefore HandleProtocol() should be
> avoided.

FWIW HandleProtocol is also obsolete so we should probably just leave
it for < 1.10 EFI apps and replace all calls with OpenProtocol (or
efi_search_protocol()) in the future

>
> * Replace the LocateHandle() call by efi_search_protocol().
> * Remove the CloseProtocol() call.
>
> Fixes: 8d99026f0697 ("efi_loader: capsule: support firmware update")
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
>  lib/efi_loader/efi_capsule.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index b6bd2d6af8..397e393a18 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -159,12 +159,14 @@ efi_fmp_find(efi_guid_t *image_type, u8 image_index, u64 instance,
>         efi_status_t ret;
>
>         for (i = 0, handle = handles; i < no_handles; i++, handle++) {
> -               ret = EFI_CALL(efi_handle_protocol(
> -                               *handle,
> -                               &efi_guid_firmware_management_protocol,
> -                               (void **)&fmp));
> +               struct efi_handler *fmp_handler;
> +
> +               ret = efi_search_protocol(
> +                               *handle, &efi_guid_firmware_management_protocol,
> +                               &fmp_handler);
>                 if (ret != EFI_SUCCESS)
>                         continue;
> +               fmp = fmp_handler->protocol_interface;
>
>                 /* get device's image info */
>                 info_size = 0;
> @@ -215,10 +217,6 @@ efi_fmp_find(efi_guid_t *image_type, u8 image_index, u64 instance,
>  skip:
>                 efi_free_pool(package_version_name);
>                 free(image_info);
> -               EFI_CALL(efi_close_protocol(
> -                               (efi_handle_t)fmp,
> -                               &efi_guid_firmware_management_protocol,
> -                               NULL, NULL));
>                 if (found)
>                         return fmp;
>         }
> --
> 2.37.2
>

With or without the nits on the commit message
Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>