[PATCH] efi_loader: esrt: Remove EFI_CALL invocation in efi_esrt_register

[Heinrich Schuchardt]

On 4/10/21 2:09 PM, Sughosh Ganu wrote:
> The efi_esrt_register function calls efi_create_event and
> efi_register_protocol_notify functions. These function calls are made
> through the EFI_CALL macro.
>
> For the Arm and RiscV architecture platforms, the EFI_CALL macro,
> before invoking the corresponding function, modifies the global_data
> pointer. Before the function calls, the gd is set to app_gd, which is
> the value used by UEFI app, and after the function call, it is
> restored back to u-boot's gd.
>
> This becomes an issue when the EFI_CALL is used for the two function
> invocations, since these functions make calls to calloc, which
> dereferences the gd pointer. With the gd pointer being no longer
> valid, this results in an abort. Since these functions are using
> u-boot's api's, they should not be called through the EFI_CALL
> macro. Fix this issue by calling these functions directly, without the
> EFI_CALL macro.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
> This issue can be seen by executing a 'printenv -e' command on an arm
> architecture platform. Executing the command results in an abort.
>
>   lib/efi_loader/efi_esrt.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
> index 947bdb5e95..141baabb01 100644
> --- a/lib/efi_loader/efi_esrt.c
> +++ b/lib/efi_loader/efi_esrt.c
> @@ -490,15 +490,15 @@ efi_status_t efi_esrt_register(void)
>   		return ret;
>   	}
>
> -	ret = EFI_CALL(efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> -					efi_esrt_new_fmp_notify, NULL, NULL, &ev));
> +	ret = efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> +			       efi_esrt_new_fmp_notify, NULL, NULL, &ev);

Dear Sughosh,

thank you for reporting and analyzing the issue.

This change is required. efi_create_event does not start with EFI_ENTRY().

>   	if (ret != EFI_SUCCESS) {
>   		EFI_PRINT("ESRT failed to create event\n");
>   		return ret;
>   	}
>
> -	ret = EFI_CALL(efi_register_protocol_notify(&efi_guid_firmware_management_protocol,
> -						    ev, &registration));
> +	ret = efi_register_protocol_notify(&efi_guid_firmware_management_protocol,
> +					   ev, &registration);

efi_register_protocol_notify() calls EFI_ENTRY() so you can only invoke
it with EFI_CALL().

scripts/get_maintainer.pl asks for adding Alex on CC.

Best regards

Heinrich

>   	if (ret != EFI_SUCCESS) {
>   		EFI_PRINT("ESRT failed to register FMP callback\n");
>   		return ret;
>