[PATCH 2/3] efi_loader: Prevent dereference of uninitialised variable

[Heinrich Schuchardt]

On 01.07.25 15:38, Andrew Goodbody 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.goodbody at 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;
> -

If ChildHandle does not support the protocol that is being removed, we 
must return EFI_UNSUPPORTED and should not continue here.
See 11.6.3 EFI_SERVICE_BINDING_PROTOCOL.DestroyChild() in the UEFI 
specification.


>   	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;

We should eliminate the variable protocol_interface.

> +		http_instance = (struct efi_http_instance *)protocol_interface;

This is not C++ code. The conversion from (void *) is superfluous.

CCing  Adriano as author of the code.

Best regards

Heinrich

> +		efi_free_pool(http_instance->http_load_addr);
> +		http_instance->http_load_addr = NULL;
>   
> -	free(protocol_interface);
> +		free(protocol_interface);
> +	}
>   
>   	num_instances--;
>   
>