Re: [PATCH 1/2] efi_loader: function to unlink udevice and handle

[Heinrich Schuchardt]

On 10/3/22 12:20, Ilias Apalodimas wrote:
Hi Heinrich,

On Mon, Oct 03, 2022 at 11:44:58AM +0200, Heinrich Schuchardt wrote:
When deleting a device or a handle we must remove the link between the two
to avoid dangling references.

Provide function efi_unlink_dev() for this purpose.

Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
---
  include/efi_loader.h        |  1 +
  lib/efi_loader/efi_helper.c | 19 +++++++++++++++++++
  2 files changed, 20 insertions(+)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index ad01395b39..5a993b0d2e 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -708,6 +708,7 @@ const char *guid_to_sha_str(const efi_guid_t *guid);
  int algo_to_len(const char *algo);
  
  int efi_link_dev(efi_handle_t handle, struct udevice *dev);
+int efi_unlink_dev(efi_handle_t handle);
  
  /**
   * efi_size_in_pages() - convert size in bytes to size in pages
diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
index 8ed564e261..c71e87d118 100644
--- a/lib/efi_loader/efi_helper.c
+++ b/lib/efi_loader/efi_helper.c
@@ -171,3 +171,22 @@ int efi_link_dev(efi_handle_t handle, struct udevice *dev)
        handle->dev = dev;
        return dev_tag_set_ptr(dev, DM_TAG_EFI, handle);
  }
+
+/**
+ * efi_unlink_dev() - unlink udevice and handle
+ *
+ * @handle:    EFI handle to unlink
+ *
+ * Return:     0 on success, negative on failure
+ */
+int efi_unlink_dev(efi_handle_t handle)
+{
+       int ret;
+
+       ret = dev_tag_del(handle->dev, DM_TAG_EFI);
+       if (ret)
+               return ret;

Is handle->dev always guaranteed to hold a valid ptr?

dev_tag_del() returns -EINVAL if !dev. We should not check twice.

Would it make sense to add an if here and use this function everywhere
instead of sprinkling dev_tag_del() around?

We should move the calls in lib/efi_loader/efi_disk.c to 
efi_delete_handle() to make sure that we clean up.

Best regards

Heinrich


+       handle->dev = NULL;
+
+       return 0;
+}
--
2.37.2


Thanks
/Ilias