Hi Andrew,
On Tue, 1 Jul 2025 at 16:38, Andrew Goodbody <andrew.goodb...@linaro.org> wrote:
>
> If phandler is returned as NULL from efi_search_protocol then
> protocol_interface is never assigned to. Add a check to prevent
> protocol_interface being dereferenced in this case. Small refactor to
> coalesce the two identical NULL checks of phandler.
>
> This issue found by Smatch.
>
> Signed-off-by: Andrew Goodbody <andrew.goodb...@linaro.org>
> ---
> lib/efi_loader/efi_http.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/lib/efi_loader/efi_http.c b/lib/efi_loader/efi_http.c
> index 189317fe2d2..ce3a7a831ca 100644
> --- a/lib/efi_loader/efi_http.c
> +++ b/lib/efi_loader/efi_http.c
> @@ -463,18 +463,18 @@ static efi_status_t EFIAPI
> efi_http_service_binding_destroy_child(
>
> efi_search_protocol(child_handle, &efi_http_guid, &phandler);
>
> - if (phandler)
> - protocol_interface = phandler->protocol_interface;
> -
> ret = efi_delete_handle(child_handle);
> if (ret != EFI_SUCCESS)
> return EFI_EXIT(ret);
>
> - http_instance = (struct efi_http_instance *)protocol_interface;
> - efi_free_pool(http_instance->http_load_addr);
> - http_instance->http_load_addr = NULL;
> + if (phandler) {
> + protocol_interface = phandler->protocol_interface;
> + http_instance = (struct efi_http_instance
> *)protocol_interface;
> + efi_free_pool(http_instance->http_load_addr);
> + http_instance->http_load_addr = NULL;
>
> - free(protocol_interface);
> + free(protocol_interface);
> + }
Reading at the spec I think we can't return EFI_SUCCESS in that case.
11.6.3 EFI_SERVICE_BINDING_PROTOCOL.DestroyChild() is the chapter that
describes the protocol.
EFI_UNSUPPORTED is supposed to be returned if the ChildHandle does not
support the protocol that is being removed.
So I think you should return a different error if phadnle is indeed NULL
Thanks
/Ilias
>
> num_instances--;
>
>
> --
> 2.39.5
>
