Hi Heinrich, On Mon, 25 Dec 2023 at 19:05, Heinrich Schuchardt <[email protected]> wrote: > > 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 <[email protected]> > > --- > > 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.
OK, I will try to improve the efi_disk_remove() function. > > > + 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. efi_delete_handle() calls efi_remove_all_protocols(), and frees the efi handle if no protocol is left on the handle. So I think efi_delete_handle() does all the required processes mentioned above. > > When a partition is removed we must close the protocol interfaces opened > with EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER (cf. efi_disk_add_dev()). OK, I will fix this. Thanks, Masahisa Kojima > > 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; >
