On 5/7/19 6:39 AM, Takahiro Akashi wrote:
On Sat, May 04, 2019 at 10:36:36AM +0200, Heinrich Schuchardt wrote:
Implement unloading of images in the Exit() boot services:
* unload images that are not yet started,
* unload started applications,
* unload drivers returning an error.
Signed-off-by: Heinrich Schuchardt <[email protected]>
---
include/efi_loader.h | 1 +
lib/efi_loader/efi_boottime.c | 34 ++++++++++++++++++++++++++-----
lib/efi_loader/efi_image_loader.c | 2 ++
3 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index c2a449e5b6..d73c89ac26 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -237,6 +237,7 @@ struct efi_loaded_image_obj {
struct jmp_buf_data exit_jmp;
EFIAPI efi_status_t (*entry)(efi_handle_t image_handle,
struct efi_system_table *st);
+ u16 image_type;
};
/**
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index bbcd66caa6..51e0bb2fd5 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -13,6 +13,7 @@
#include <linux/libfdt_env.h>
#include <u-boot/crc.h>
#include <bootm.h>
+#include <pe.h>
#include <watchdog.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -2798,7 +2799,7 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t
image_handle,
* image protocol.
*/
efi_status_t ret;
- void *info;
+ struct efi_loaded_image *loaded_image_protocol;
struct efi_loaded_image_obj *image_obj =
(struct efi_loaded_image_obj *)image_handle;
@@ -2806,13 +2807,33 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t
image_handle,
exit_data_size, exit_data);
/* Check parameters */
- if (image_handle != current_image)
+ if (image_handle != current_image) {
Does this check make sense even for a driver?
The check is prescribed in the UEFI specification. See the heading
"Status Codes Returned".
Multiple binaries may be in status started. If a child image (which may
be a driver) calls Exit() with the parent image handle this might cause
fancy problems.
The lifetime of a driver is LoadImage(), StartImage(), Exit(),
UnloadImage(). Typically between StartImage() and Exit() it will install
its protocols and between Exit() and UnloadImage() wait for other
binaries to call the protocols.
+ ret = EFI_INVALID_PARAMETER;
goto out;
+ }
ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image,
- &info, NULL, NULL,
+ (void **)&loaded_image_protocol,
+ NULL, NULL,
EFI_OPEN_PROTOCOL_GET_PROTOCOL));
- if (ret != EFI_SUCCESS)
+ if (ret != EFI_SUCCESS) {
Is this check necessary even when 'header.type' is introduced?
I am passing the loaded image protocol to efi_delete_handle(). In
principal a binary might have uninstalled its own loaded image protocol
so this call may fail.
+ ret = EFI_INVALID_PARAMETER;
goto out;
+ }
+
+ /* Unloading of unstarted images */
'Unload' sounds odd when we call efi_delete_image(), doesn't it?
+ switch (image_obj->header.type) {
+ case EFI_OBJECT_TYPE_STARTED_IMAGE:
+ break;
+ case EFI_OBJECT_TYPE_LOADED_IMAGE:
+ efi_delete_image(image_obj, loaded_image_protocol);
+ ret = EFI_SUCCESS;
+ goto out;
+ default:
+ /* Handle does not refer to loaded image */
+ ret = EFI_INVALID_PARAMETER;
+ goto out;
+ }
+ image_obj->header.type = EFI_OBJECT_TYPE_LOADED_IMAGE;
/* Exit data is only foreseen in case of failure. */
if (exit_status != EFI_SUCCESS) {
@@ -2822,6 +2843,9 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t
image_handle,
if (ret != EFI_SUCCESS)
EFI_PRINT("%s: out of memory\n", __func__);
}
+ if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION ||
+ exit_status != EFI_SUCCESS)
+ efi_delete_image(image_obj, loaded_image_protocol);
Why not merge those two efi_delete_image()?
I went for readable code. The if statement catching all cases was
unreadable to me.
Best regards
Heinrich
-Takahiro Akashi
/* Make sure entry/exit counts for EFI world cross-overs match */
EFI_EXIT(exit_status);
@@ -2837,7 +2861,7 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t
image_handle,
panic("EFI application exited");
out:
- return EFI_EXIT(EFI_INVALID_PARAMETER);
+ return EFI_EXIT(ret);
}
/**
diff --git a/lib/efi_loader/efi_image_loader.c
b/lib/efi_loader/efi_image_loader.c
index f8092b6202..13541cfa7a 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -273,6 +273,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj
*handle, void *efi,
IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
image_base = opt->ImageBase;
efi_set_code_and_data_type(loaded_image_info, opt->Subsystem);
+ handle->image_type = opt->Subsystem;
efi_reloc = efi_alloc(virt_size,
loaded_image_info->image_code_type);
if (!efi_reloc) {
@@ -288,6 +289,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj
*handle, void *efi,
IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
image_base = opt->ImageBase;
efi_set_code_and_data_type(loaded_image_info, opt->Subsystem);
+ handle->image_type = opt->Subsystem;
efi_reloc = efi_alloc(virt_size,
loaded_image_info->image_code_type);
if (!efi_reloc) {
--
2.20.1
_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot
