[U-Boot] [PATCH 0/2] efi_loader: parameter checks in StartImage and Exit()
Add parameter checks in the StartImage() and Exit() boottime services: - check that the image handle is valid and has the loaded image protocol installed - in StartImage() record the current image - in Exit() check that the image is the current image
Heinrich Schuchardt (2): efi_loader: rearrange boottime service functions efi_loader: parameter checks in StartImage and Exit()
lib/efi_loader/efi_boottime.c | 246 +++++++++++++++++++--------------- 1 file changed, 137 insertions(+), 109 deletions(-)
-- 2.20.1
To avoid forward declarations move efi_start_image() and efi_exit() down.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 218 +++++++++++++++++----------------- 1 file changed, 109 insertions(+), 109 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index cb75e81155..9376d336ab 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1744,115 +1744,6 @@ error: return EFI_EXIT(ret); }
-/** - * efi_start_image() - call the entry point of an image - * @image_handle: handle of the image - * @exit_data_size: size of the buffer - * @exit_data: buffer to receive the exit data of the called image - * - * This function implements the StartImage service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * Return: status code - */ -efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, - efi_uintn_t *exit_data_size, - u16 **exit_data) -{ - struct efi_loaded_image_obj *image_obj = - (struct efi_loaded_image_obj *)image_handle; - efi_status_t ret; - - EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data); - - efi_is_direct_boot = false; - - /* call the image! */ - if (setjmp(&image_obj->exit_jmp)) { - /* - * We called the entry point of the child image with EFI_CALL - * in the lines below. The child image called the Exit() boot - * service efi_exit() which executed the long jump that brought - * us to the current line. This implies that the second half - * of the EFI_CALL macro has not been executed. - */ -#ifdef CONFIG_ARM - /* - * efi_exit() called efi_restore_gd(). We have to undo this - * otherwise __efi_entry_check() will put the wrong value into - * app_gd. - */ - gd = app_gd; -#endif - /* - * To get ready to call EFI_EXIT below we have to execute the - * missed out steps of EFI_CALL. - */ - assert(__efi_entry_check()); - debug("%sEFI: %lu returned by started image\n", - __efi_nesting_dec(), - (unsigned long)((uintptr_t)image_obj->exit_status & - ~EFI_ERROR_MASK)); - return EFI_EXIT(image_obj->exit_status); - } - - ret = EFI_CALL(image_obj->entry(image_handle, &systab)); - - /* - * Usually UEFI applications call Exit() instead of returning. - * But because the world doesn't consist of ponies and unicorns, - * we're happy to emulate that behavior on behalf of a payload - * that forgot. - */ - return EFI_CALL(systab.boottime->exit(image_handle, ret, 0, NULL)); -} - -/** - * efi_exit() - leave an EFI application or driver - * @image_handle: handle of the application or driver that is exiting - * @exit_status: status code - * @exit_data_size: size of the buffer in bytes - * @exit_data: buffer with data describing an error - * - * This function implements the Exit service. - * - * See the Unified Extensible Firmware Interface (UEFI) specification for - * details. - * - * Return: status code - */ -static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, - efi_status_t exit_status, - efi_uintn_t exit_data_size, - u16 *exit_data) -{ - /* - * TODO: We should call the unload procedure of the loaded - * image protocol. - */ - struct efi_loaded_image_obj *image_obj = - (struct efi_loaded_image_obj *)image_handle; - - EFI_ENTRY("%p, %ld, %zu, %p", image_handle, exit_status, - exit_data_size, exit_data); - - /* Make sure entry/exit counts for EFI world cross-overs match */ - EFI_EXIT(exit_status); - - /* - * But longjmp out with the U-Boot gd, not the application's, as - * the other end is a setjmp call inside EFI context. - */ - efi_restore_gd(); - - image_obj->exit_status = exit_status; - longjmp(&image_obj->exit_jmp, 1); - - panic("EFI application exited"); -} - /** * efi_unload_image() - unload an EFI image * @image_handle: handle of the image to be unloaded @@ -2720,6 +2611,115 @@ out: return EFI_EXIT(r); }
+/** + * efi_start_image() - call the entry point of an image + * @image_handle: handle of the image + * @exit_data_size: size of the buffer + * @exit_data: buffer to receive the exit data of the called image + * + * This function implements the StartImage service. + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * Return: status code + */ +efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, + efi_uintn_t *exit_data_size, + u16 **exit_data) +{ + struct efi_loaded_image_obj *image_obj = + (struct efi_loaded_image_obj *)image_handle; + efi_status_t ret; + + EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data); + + efi_is_direct_boot = false; + + /* call the image! */ + if (setjmp(&image_obj->exit_jmp)) { + /* + * We called the entry point of the child image with EFI_CALL + * in the lines below. The child image called the Exit() boot + * service efi_exit() which executed the long jump that brought + * us to the current line. This implies that the second half + * of the EFI_CALL macro has not been executed. + */ +#ifdef CONFIG_ARM + /* + * efi_exit() called efi_restore_gd(). We have to undo this + * otherwise __efi_entry_check() will put the wrong value into + * app_gd. + */ + gd = app_gd; +#endif + /* + * To get ready to call EFI_EXIT below we have to execute the + * missed out steps of EFI_CALL. + */ + assert(__efi_entry_check()); + debug("%sEFI: %lu returned by started image\n", + __efi_nesting_dec(), + (unsigned long)((uintptr_t)image_obj->exit_status & + ~EFI_ERROR_MASK)); + return EFI_EXIT(image_obj->exit_status); + } + + ret = EFI_CALL(image_obj->entry(image_handle, &systab)); + + /* + * Usually UEFI applications call Exit() instead of returning. + * But because the world doesn't consist of ponies and unicorns, + * we're happy to emulate that behavior on behalf of a payload + * that forgot. + */ + return EFI_CALL(systab.boottime->exit(image_handle, ret, 0, NULL)); +} + +/** + * efi_exit() - leave an EFI application or driver + * @image_handle: handle of the application or driver that is exiting + * @exit_status: status code + * @exit_data_size: size of the buffer in bytes + * @exit_data: buffer with data describing an error + * + * This function implements the Exit service. + * + * See the Unified Extensible Firmware Interface (UEFI) specification for + * details. + * + * Return: status code + */ +static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, + efi_status_t exit_status, + efi_uintn_t exit_data_size, + u16 *exit_data) +{ + /* + * TODO: We should call the unload procedure of the loaded + * image protocol. + */ + struct efi_loaded_image_obj *image_obj = + (struct efi_loaded_image_obj *)image_handle; + + EFI_ENTRY("%p, %ld, %zu, %p", image_handle, exit_status, + exit_data_size, exit_data); + + /* Make sure entry/exit counts for EFI world cross-overs match */ + EFI_EXIT(exit_status); + + /* + * But longjmp out with the U-Boot gd, not the application's, as + * the other end is a setjmp call inside EFI context. + */ + efi_restore_gd(); + + image_obj->exit_status = exit_status; + longjmp(&image_obj->exit_jmp, 1); + + panic("EFI application exited"); +} + /** * efi_handle_protocol() - get interface of a protocol on a handle * @handle: handle on which the protocol shall be opened -- 2.20.1
Add parameter checks in the StartImage() and Exit() boottime services: - check that the image handle is valid and has the loaded image protocol installed - in StartImage() record the current image - in Exit() check that the image is the current image
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 9376d336ab..fb5d88e678 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -26,6 +26,9 @@ LIST_HEAD(efi_obj_list); /* List of all events */ LIST_HEAD(efi_events);
+/* Handle of the currently executing image */ +static efi_handle_t current_image; + /* * If we're running on nasty systems (32bit ARM booting into non-EFI Linux) * we need to do trickery with caches. Since we don't want to break the EFI @@ -2631,9 +2634,18 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, struct efi_loaded_image_obj *image_obj = (struct efi_loaded_image_obj *)image_handle; efi_status_t ret; + void *info; + efi_handle_t parent_image;
EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data);
+ /* Check parameters */ + ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image, + &info, NULL, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL)); + if (ret != EFI_SUCCESS) + return EFI_EXIT(EFI_INVALID_PARAMETER); + efi_is_direct_boot = false;
/* call the image! */ @@ -2662,9 +2674,12 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, __efi_nesting_dec(), (unsigned long)((uintptr_t)image_obj->exit_status & ~EFI_ERROR_MASK)); + current_image = parent_image; return EFI_EXIT(image_obj->exit_status); }
+ parent_image = current_image; + current_image = image_handle; ret = EFI_CALL(image_obj->entry(image_handle, &systab));
/* @@ -2699,12 +2714,23 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, * TODO: We should call the unload procedure of the loaded * image protocol. */ + efi_status_t ret; + void *info; struct efi_loaded_image_obj *image_obj = (struct efi_loaded_image_obj *)image_handle;
EFI_ENTRY("%p, %ld, %zu, %p", image_handle, exit_status, exit_data_size, exit_data);
+ /* Check parameters */ + if (image_handle != current_image) + goto out; + ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image, + &info, NULL, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL)); + if (ret != EFI_SUCCESS) + goto out; + /* Make sure entry/exit counts for EFI world cross-overs match */ EFI_EXIT(exit_status);
@@ -2718,6 +2744,8 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, longjmp(&image_obj->exit_jmp, 1);
panic("EFI application exited"); +out: + return EFI_EXIT(EFI_INVALID_PARAMETER); }
/** -- 2.20.1
On 27.03.19 01:41, Heinrich Schuchardt wrote:
Add parameter checks in the StartImage() and Exit() boottime services:
- check that the image handle is valid and has the loaded image protocol installed
- in StartImage() record the current image
- in Exit() check that the image is the current image
Could you please elaborate what the checks are for? Are they mandated by the spec? Or did you spot them missing in a real world scenario where everything fell apart without you noticing before?
I'm slightly concerned by the amount of runtime sanity checks we add to the code. It's a lot of bloat (binary size as well as execution time) for questionable gain outside of debug builds.
Alex
On 27.03.19 01:41, Heinrich Schuchardt wrote:
Add parameter checks in the StartImage() and Exit() boottime services:
- check that the image handle is valid and has the loaded image protocol installed
- in StartImage() record the current image
- in Exit() check that the image is the current image
Could you please elaborate what the checks are for? Are they mandated by the spec? Or did you spot them missing in a real world scenario where everything fell apart without you noticing before?
I'm slightly concerned by the amount of runtime sanity checks we add to the code. It's a lot of bloat (binary size as well as execution time) for questionable gain outside of debug builds.
Alex
participants (2)
-
Alexander Graf -
Heinrich Schuchardt