[U-Boot] [RFC 1/8] efi_loader: boottime: don't add device path protocol to image handle

[Heinrich Schuchardt]

On 3/6/19 6:04 AM, Heinrich Schuchardt wrote:
> On 3/6/19 1:27 AM, AKASHI Takahiro wrote:
>> On Tue, Mar 05, 2019 at 08:48:37PM +0100, Heinrich Schuchardt wrote:
>>> On 3/5/19 6:53 AM, AKASHI Takahiro wrote:
>>>> It is just wrong to add devcie path protocol to image handle.
>>>>
>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>>> ---
>>>>  lib/efi_loader/efi_boottime.c | 11 +----------
>>>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>>>
>>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>>>> index bd8b8a17ae71..7bd9c0a952d4 100644
>>>> --- a/lib/efi_loader/efi_boottime.c
>>>> +++ b/lib/efi_loader/efi_boottime.c
>>>> @@ -1540,17 +1540,8 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path,
>>>>  	info->file_path = file_path;
>>>>  	info->system_table = &systab;
>>>>  
>>>> -	if (device_path) {
>>>> +	if (device_path)
>>>>  		info->device_handle = efi_dp_find_obj(device_path, NULL);
>>>> -		/*
>>>> -		 * When asking for the device path interface, return
>>>> -		 * bootefi_device_path
>>>> -		 */
>>>> -		ret = efi_add_protocol(&obj->header,
>>>> -				       &efi_guid_device_path, device_path);
>>>
>>> Installing the device path is not the problem. It is the GUID that is
>>> wrong. Use EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL_GUID here.
>>
>> Okay, but I believe that we need duplicate device_path here
>> before installing it as EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL_GUID.
>>
>> See this line:
>>
>>>>  		info->device_handle = efi_dp_find_obj(device_path, NULL);
>>
>> Normally, device_path is expected to be already associated with
>> another handle. We should not allow two handles to share one protocol(data).
>> That is also why I initially believed that add_protocol() should be removed.
> 
> The spec says we should use a copy of the unchanged DevicePath parameter
> of LoadImage() which may be NULL.
> 
> So we have to rework all callers to get the device_path parameter of
> efi_setup_loaded_image() right.
> 

Why are we constructing a dummy memory device path at all in cmd/bootefi?

The commit message of patch bf19273e81eb "efi_loader: Add mem-mapped for
fallback" that introduced this does not give a valid answer as it is
explicitly allowable to call LoadImage with DevicePath = NULL if
SourceBuffer is provided.

So I suggest we rid ourselves of the dummy device path with this patch
series.

Best regards

Heinrich