[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.schuchardt at 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