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

Heinrich Schuchardt
2019-03-06T05:29:14Z

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@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