From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takahiro Akashi Date: Wed, 8 May 2019 10:08:12 +0900 Subject: [U-Boot] [PATCH v2 1/1] efi_loader: unload applications upon Exit() In-Reply-To: <7eebda15-e316-0c70-476e-007a58afad98@gmx.de> References: <20190507191324.13283-1-xypron.glpk@gmx.de> <20190507235909.GK11160@linaro.org> <7eebda15-e316-0c70-476e-007a58afad98@gmx.de> Message-ID: <20190508010811.GL11160@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, May 08, 2019 at 02:59:08AM +0200, Heinrich Schuchardt wrote: > On 5/8/19 1:59 AM, Takahiro Akashi wrote: > >On Tue, May 07, 2019 at 09:13:24PM +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 > >>--- > >>v2 > >> Images that are no yet started can be unloaded by calling Exit(). > >> In this case they are not the current image. Move the test for > >> current down in the code. > >> > >> A started driver that called Exit() should still be considered a > >> started image. Exit cannot be called by another image afterwards, > >> cf. UEFI SCT 2.6 (2017), 3.5.1 Exit(), 5.1.4.5.8 - 5.1.4.5.10. > >>--- > >> include/efi_loader.h | 1 + > >> lib/efi_loader/efi_boottime.c | 36 +++++++++++++++++++++++++------ > >> lib/efi_loader/efi_image_loader.c | 2 ++ > >> 3 files changed, 33 insertions(+), 6 deletions(-) > >> > >>diff --git a/include/efi_loader.h b/include/efi_loader.h > >>index 3b50cd28ef..4e4cffa799 100644 > >>--- a/include/efi_loader.h > >>+++ b/include/efi_loader.h > >>@@ -234,6 +234,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 0385883ded..1ea96dab6c 100644 > >>--- a/lib/efi_loader/efi_boottime.c > >>+++ b/lib/efi_loader/efi_boottime.c > >>@@ -13,6 +13,7 @@ > >> #include > >> #include > >> #include > >>+#include > >> #include > >> > >> 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) > >>- 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) { > >>+ ret = EFI_INVALID_PARAMETER; > >> goto out; > >>+ } > >>+ > >>+ /* Unloading of unstarted images */ > >>+ 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; > >>+ } > >>+ /* A started image can only be unloaded it is the last one started. */ > >>+ if (image_handle != current_image) { > >>+ ret = EFI_INVALID_PARAMETER; > >>+ goto out; > >>+ } > >> > >> /* 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); > > > >No change around efi_delete_image() and "goto" above? > > > > Do you see a bug? > > A diff would help me to understand what you would like to change. You said: >> For me, your code is much unreadable. >> Moreover, I remember that you have said, in a review of my patch, that >> we should use "goto" only in error cases. > >Good point. So the check must be after handling >EFI_OBJECT_TYPE_LOADED_IMAGE. > >I will revise the patch. -Takahiro Akashi > Best regards > > Heinrich >