Re: [PATCH 2/3] efi_loader: avoid pointer access after calling efi_delete_handle

[Heinrich Schuchardt]

On 12/25/23 05:43, Masahisa Kojima wrote:

efi_delete_handle() calls efi_purge_handle(), then it finally
frees the efi handle.
Both diskobj and handle variables in efi_disk_remove() have
the same pointer, we can not access diskobj->dp after calling
efi_delete_handle().

This commit saves the struct efi_device_path pointer before
calling efi_delete_handle(). This commit also fixes the
missing free for volume member in struct efi_disk_obj.

Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
---
  lib/efi_loader/efi_disk.c | 12 +++++++++---
  1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index a2f8b531a3..415d8601ba 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -701,7 +701,9 @@ int efi_disk_remove(void *ctx, struct event *event)
        struct udevice *dev = event->data.dm.dev;
        efi_handle_t handle;
        struct blk_desc *desc;
+       struct efi_device_path *dp = NULL;
        struct efi_disk_obj *diskobj = NULL;
+       struct efi_simple_file_system_protocol *volume = NULL;
        efi_status_t ret;

        if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))
@@ -722,14 +724,18 @@ int efi_disk_remove(void *ctx, struct event *event)
                return 0;
        }

+       if (diskobj) {

  diskobj = container_of(handle, struct efi_disk_obj, header);

can be replaced by

  diskobj = handle

We should check the handle immediately after dev_tag_get_ptr() and
return 0 if the handle == NULL.

+               dp = diskobj->dp;
+               volume = diskobj->volume;
+       }
+
        ret = efi_delete_handle(handle);

We must not delete the handle in case of UCLASS_EFI_LOADER.

Instead of calling efi_delete_handle we should uninstall all protocols
that we have installed. If no protocol is left the handle will go away.

To make the protocols to delete tractable they should be opened with
BY_DRIVER.

When a partition is removed we must close the protocol interfaces opened
with EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER (cf. efi_disk_add_dev()).

Best regards

Heinrich

        /* 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_free_pool(dp);
+       free(volume);
        dev_tag_del(dev, DM_TAG_EFI);

        return 0;