[...]
+/**
- efi_handle_cleanup() - Clean the deleted handle from the various lists
- @handle: handle to remove
- @force: force removal even if protocols are still installed on the handle
- Return: status code
- */
+static void efi_handle_cleanup(efi_handle_t handle, bool force) +{
- struct efi_register_notify_event *item, *next;
- if (!list_empty(&handle->protocols) || force)
I guess the parameter force can be dropped from the interface.
'force' is here for a reason(see below), although the if is wrong.
The correct one is obviously if (!list_empty(&handle->protocols) && !force) ....
return;Please, return a failure code.
I am not sure why you want that, but I'll have a closer look
- /* The handle is about to be freed. Remove it from events */
- list_for_each_entry_safe(item, next, &efi_register_notify_events, link) {
struct efi_protocol_notification *hitem, *hnext;list_for_each_entry_safe(hitem, hnext, &item->handles, link) {if (handle == hitem->handle) {list_del(&hitem->link);free(hitem);}}- }
- /* The last protocol has been removed, delete the handle. */
- list_del(&handle->link);
- free(handle);
+}
- /**
*/
- efi_process_event_queue() - process event queue
@@ -627,8 +656,7 @@ void efi_delete_handle(efi_handle_t handle) return; }
It would be safer to move the checks if a protocol interface can be removed from efi_uninstall_protocol() to efi_remove_protocol(). That way we are sure that no driver has attached to the protocols that we try to remove.
Yes it would, in fact I tried this on my initial patches and I mention it on the patch description . However the selftests started failing. The reason is that efi_reinstall_protocol_interface() does not remove the handle and we have specific selftests for that. I don't really know or remember why that was done like that, does the EFI spec describe this? I thought UninstallProtocolInterface was always supposed to delete the handle if it had no more protocols
- list_del(&handle->link);
- free(handle);
- efi_handle_cleanup(handle, true);
I don't think we should use a special treatment here. If the protocols cannot be deleted, because a driver has attached we should not delete the handle.
The function should return a status code. Then efi_disk_delete_raw() can return an error for EVT_DM_PRE_REMOVE and stop the device from being deleted.
We agree again, but I am not trying to fix that atm. The reason for the special check and the force flag is that efi_delete_handle() is already doing something similar, which is cwnot optimal, but that's the current state. It bails out only if the *handle* doesn't exist and doesn't care if the protocols got removed. If you prefer having a bigger cleanup + fixes let me know.
Thanks /Ilias
Best regards
Heinrich
}
/** @@ -1396,11 +1424,8 @@ static efi_status_t EFIAPI efi_uninstall_protocol_interface if (ret != EFI_SUCCESS) goto out;
- /* If the last protocol has been removed, delete the handle. */
- if (list_empty(&handle->protocols)) {
list_del(&handle->link);free(handle);- }
- /* Check if we have to purge the handle from various lists */
- efi_handle_cleanup(handle, false); out: return EFI_EXIT(ret); }
@@ -2777,11 +2802,7 @@ efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle, i++; } if (ret == EFI_SUCCESS) {
/* If the last protocol has been removed, delete the handle. */if (list_empty(&handle->protocols)) {list_del(&handle->link);free(handle);}
efi_handle_cleanup(handle, false);