[v2] efi_loader: esrt: Remove EFI_CALL invocation for efi_create_event

Message ID 20210410150948.24240-1-sughosh.ganu@linaro.org
State New
Headers show
Series
  • [v2] efi_loader: esrt: Remove EFI_CALL invocation for efi_create_event
Related show

Commit Message

Sughosh Ganu April 10, 2021, 3:09 p.m.
The efi_esrt_register function calls the efi_create_event function
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 function
invocation, since the function makes calls to calloc, which
dereferences the gd pointer. With the gd pointer being no longer
valid, this results in an abort. Since this function is using
u-boot's api's, it should not be called through the EFI_CALL
macro. Fix this issue by calling the function directly, without the
EFI_CALL macro.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>

---

Changes since V1:
Remove the EFI_CALL macro only for efi_create_event function
invocation, and not for the efi_register_protocol_notify invocation,
since the later has an EFI_ENTRY function call.

 lib/efi_loader/efi_esrt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.17.1

Comments

Heinrich Schuchardt April 10, 2021, 4:55 p.m. | #1
On 4/10/21 5:09 PM, Sughosh Ganu wrote:
> The efi_esrt_register function calls the efi_create_event function

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

> invocation, since the function makes calls to calloc, which

> dereferences the gd pointer. With the gd pointer being no longer

> valid, this results in an abort. Since this function is using

> u-boot's api's, it should not be called through the EFI_CALL

> macro. Fix this issue by calling the function directly, without the

> EFI_CALL macro.

>

> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>


Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


> ---

>

> Changes since V1:

> Remove the EFI_CALL macro only for efi_create_event function

> invocation, and not for the efi_register_protocol_notify invocation,

> since the later has an EFI_ENTRY function call.

>

>   lib/efi_loader/efi_esrt.c | 4 ++--

>   1 file changed, 2 insertions(+), 2 deletions(-)

>

> diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c

> index 947bdb5e95..461bd4b937 100644

> --- a/lib/efi_loader/efi_esrt.c

> +++ b/lib/efi_loader/efi_esrt.c

> @@ -490,8 +490,8 @@ 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);

>   	if (ret != EFI_SUCCESS) {

>   		EFI_PRINT("ESRT failed to create event\n");

>   		return ret;

>

Patch

diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
index 947bdb5e95..461bd4b937 100644
--- a/lib/efi_loader/efi_esrt.c
+++ b/lib/efi_loader/efi_esrt.c
@@ -490,8 +490,8 @@  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);
 	if (ret != EFI_SUCCESS) {
 		EFI_PRINT("ESRT failed to create event\n");
 		return ret;