efi_loader: make efi_delete_handle() follow the EFI spec

The EFI doesn't allow removal of handles, unless all hosted protocols
are cleanly removed.  Our efi_delete_handle() is a bit intrusive.
Although it does try to delete protocols before removing a handle,
it doesn't care if that fails.  Instead it only returns an error if the
handle is invalid. On top of that none of the callers of that function
check the return code.

So let's rewrite this in a way that fits the EFI spec better.  Instead
of forcing the handle removal, gracefully uninstall all the handle
protocols.  According to the EFI spec when the last protocol is removed
the handle will be deleted.  Also switch all the callers and check the
return code. Some callers can't do anything useful apart from reporting
an error.  The disk related functions on the other hand, can prevent a
medium that is being used by EFI from removal.

The only function that doesn't check the result is efi_delete_image().
But that function needs a bigger rework anyway, so we can clean it up in
the future

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 5c0afec1544d96fff08d9664d00d391ae45f9010..f73d6eb0e2d83570ca654c4076af4fed9db25c19 100644 (file)
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -606,20 +606,6 @@ failure:
        return ret;
 }
 
-/**
- * bootefi_run_finish() - finish up after running an EFI test
- *
- * @loaded_image_info: Pointer to a struct which holds the loaded image info
- * @image_obj: Pointer to a struct which holds the loaded image object
- */
-static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj,
-                              struct efi_loaded_image *loaded_image_info)
-{
-       efi_restore_gd();
-       free(loaded_image_info->load_options);
-       efi_delete_handle(&image_obj->header);
-}
-
 /**
  * do_efi_selftest() - execute EFI selftest
  *
@@ -638,7 +624,12 @@ static int do_efi_selftest(void)
 
        /* Execute the test */
        ret = EFI_CALL(efi_selftest(&image_obj->header, &systab));
-       bootefi_run_finish(image_obj, loaded_image_info);
+       efi_restore_gd();
+       free(loaded_image_info->load_options);
+       if (ret != EFI_SUCCESS)
+               efi_delete_handle(&image_obj->header);
+       else
+               ret = efi_delete_handle(&image_obj->header);
 
        return ret != EFI_SUCCESS;
 }

diff --git a/include/efi_loader.h b/include/efi_loader.h
index b5fa0fe01dedfdcdcd9ff7c947ead83c552ad4d6..3a64eb9c66722c4c1651320982d80059f59bb48c 100644 (file)
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -629,7 +629,7 @@ void efi_add_handle(efi_handle_t obj);
 /* Create handle */
 efi_status_t efi_create_handle(efi_handle_t *handle);
 /* Delete handle */
-void efi_delete_handle(efi_handle_t obj);
+efi_status_t efi_delete_handle(efi_handle_t obj);
 /* Call this to validate a handle and find the EFI object for it */
 struct efi_object *efi_search_obj(const efi_handle_t handle);
 /* Locate device_path handle */

diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
index 45f935198874a0cae23ea301ce14092bbc3275de..66a45e156d60fb85f79d38089d832a29e6abfb57 100644 (file)
--- a/lib/efi_driver/efi_uclass.c
+++ b/lib/efi_driver/efi_uclass.c
@@ -285,10 +285,8 @@ static efi_status_t efi_add_driver(struct driver *drv)
        bp->ops = ops;
 
        ret = efi_create_handle(&bp->bp.driver_binding_handle);
-       if (ret != EFI_SUCCESS) {
-               free(bp);
-               goto out;
-       }
+       if (ret != EFI_SUCCESS)
+               goto err;
        bp->bp.image_handle = bp->bp.driver_binding_handle;
        ret = efi_add_protocol(bp->bp.driver_binding_handle,
                               &efi_guid_driver_binding_protocol, bp);
@@ -299,11 +297,11 @@ static efi_status_t efi_add_driver(struct driver *drv)
                if (ret != EFI_SUCCESS)
                        goto err;
        }
-out:
-       return ret;
 
+       return ret;
 err:
-       efi_delete_handle(bp->bp.driver_binding_handle);
+       if (bp->bp.driver_binding_handle)
+               efi_delete_handle(bp->bp.driver_binding_handle);
        free(bp);
        return ret;
 }

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 052fe481e473a5067bb3c40b22e6592b096c4d5b..0e89c8505b11e8e4ad833c1bb7dae02f1eb7474d 100644 (file)
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -59,6 +59,10 @@ static efi_handle_t current_image;
 static volatile gd_t *efi_gd, *app_gd;
 #endif
 
+static efi_status_t efi_uninstall_protocol
+                       (efi_handle_t handle, const efi_guid_t *protocol,
+                        void *protocol_interface);
+
 /* 1 if inside U-Boot code, 0 if inside EFI payload code */
 static int entry_count = 1;
 static int nesting_level;
@@ -610,8 +614,8 @@ static efi_status_t efi_remove_all_protocols(const efi_handle_t handle)
        list_for_each_entry_safe(protocol, pos, &efiobj->protocols, link) {
                efi_status_t ret;
 
-               ret = efi_remove_protocol(handle, &protocol->guid,
-                                         protocol->protocol_interface);
+               ret = efi_uninstall_protocol(handle, &protocol->guid,
+                                            protocol->protocol_interface);
                if (ret != EFI_SUCCESS)
                        return ret;
        }
@@ -622,19 +626,23 @@ static efi_status_t efi_remove_all_protocols(const efi_handle_t handle)
  * efi_delete_handle() - delete handle
  *
  * @handle: handle to delete
+ *
+ * Return: status code
  */
-void efi_delete_handle(efi_handle_t handle)
+efi_status_t efi_delete_handle(efi_handle_t handle)
 {
        efi_status_t ret;
 
        ret = efi_remove_all_protocols(handle);
-       if (ret == EFI_INVALID_PARAMETER) {
-               log_err("Can't remove invalid handle %p\n", handle);
-               return;
+       if (ret != EFI_SUCCESS) {
+               log_err("Handle %p has protocols installed. Unable to delete\n", handle);
+               return ret;
        }
 
        list_del(&handle->link);
        free(handle);
+
+       return ret;
 }
 
 /**

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 28c8cdf7100edfd2bb09a30352aa6d749b32e2e0..46cb5704a729e0b659c082dc2a1f2601010d4729 100644 (file)
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -708,6 +708,7 @@ int efi_disk_remove(void *ctx, struct event *event)
        efi_handle_t handle;
        struct blk_desc *desc;
        struct efi_disk_obj *diskobj = NULL;
+       efi_status_t ret;
 
        if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))
                return 0;
@@ -727,10 +728,14 @@ int efi_disk_remove(void *ctx, struct event *event)
                return 0;
        }
 
+       ret = efi_delete_handle(handle);
+       /* Do not delete DM device if there are still EFI drivers attached. */
+       if (ret != EFI_SUCCESS)
+               return -1;
+
        if (diskobj)
                efi_free_pool(diskobj->dp);
 
-       efi_delete_handle(handle);
        dev_tag_del(dev, DM_TAG_EFI);
 
        return 0;