[PATCH v2] efi_loader: unload image if efi_run_image() fails before starting it

[Yuho Choi <oss.patchbox@gmail.com>]

efi_run_image() returns without unloading the image in two cases where
the image has been loaded:

- efi_load_image() returns EFI_SECURITY_VIOLATION when an image fails
  Secure Boot authentication. The image has still been loaded and a
  valid image handle is returned, and it is up to the caller to unload
  it.
- efi_env_set_load_options() fails after the image has been loaded.

Each such attempt to run an image through 'bootefi', 'bootm' or an EFI
bootflow leaves the relocated image pages, the image handle and its
loaded image protocol behind.

Unload the image on any error before do_bootefi_exec() is called. For
its other errors efi_load_image() either sets the handle to NULL or
fails before it creates one, so initialize the handle to NULL and only
unload when it is set.

Fixes: 6b95b38c41a6 ("efi_loader: rework bootmgr/bootefi using load_image API")
Fixes: 4540dabdcaca ("efi_loader: image_loader: support image authentication")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
Changes in v2:
- Unload the image on any error before it is started, not only on
  EFI_SECURITY_VIOLATION (Heinrich)
- Add a Fixes tag for the efi_env_set_load_options() path
- Reword the subject and the commit message accordingly

Tested on sandbox only (LTO disabled, gdb). Forcing either a Secure
Boot authentication failure (efi_secure_boot set to true in
efi_load_image()) or an efi_set_load_options() failure for three
'bootefi hello' runs left 8 handles in 'efidebug dh' without this
patch, 3 of them with a Loaded Image protocol; with it the count stays
at 5. A normal 'bootefi hello' still runs and gets its load options.
Not tested with enrolled keys or on hardware.

do_bootefi_exec() also leaves the image loaded when efi_set_watchdog()
fails before efi_start_image(). That affects all of its callers, so I
will look at it separately.

 lib/efi_loader/efi_bootbin.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c
index 12db8f517c7..5bcb8c2540f 100644
--- a/lib/efi_loader/efi_bootbin.c
+++ b/lib/efi_loader/efi_bootbin.c
@@ -170,7 +170,7 @@ static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size,
 				  struct efi_device_path *dp_dev,
 				  struct efi_device_path *dp_img)
 {
-	efi_handle_t handle;
+	efi_handle_t handle = NULL;
 	struct efi_device_path *msg_path, *file_path;
 	efi_status_t ret;
 	u16 *load_options;
@@ -192,9 +192,15 @@ static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size,
 	if (ret != EFI_SUCCESS)
 		goto out;
 
-	ret = do_bootefi_exec(handle, load_options);
+	return do_bootefi_exec(handle, load_options);
 
 out:
+	/*
+	 * The image has not been started. efi_load_image() also returns a
+	 * handle for an image that fails authentication.
+	 */
+	if (handle && EFI_CALL(efi_unload_image(handle)) != EFI_SUCCESS)
+		log_err("Unloading image failed\n");
 
 	return ret;
 }
-- 
2.43.0