[U-Boot] [PATCH 1/1] efi_loader: do not add \ in efi_fs_from_name()

[Alexander Graf]

On 23.02.19 18:27, Heinrich Schuchardt wrote:
> On 2/23/19 2:37 PM, Alexander Graf wrote:
>>
>>
>>> Am 23.02.2019 um 02:20 schrieb Heinrich Schuchardt <xypron.glpk at gmx.de>:
>>>
>>> In the `efidebug boot add` command we do not want a unsollicitated leading
>>> backslash added to the file name.
>>>
>>> There is no good reason to mark a loaded file with a backslash as absolute.
>>> Anyway when reading files the file name will be interpreted as relative to
>>> root directory of the device.
>>>
>>> So let's get rid of this backslash.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>> lib/efi_loader/efi_device_path.c | 7 +------
>>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
>>> index 98c36e798f5..53b40c8c3c2 100644
>>> --- a/lib/efi_loader/efi_device_path.c
>>> +++ b/lib/efi_loader/efi_device_path.c
>>> @@ -985,12 +985,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
>>>    if (!path)
>>>        return EFI_SUCCESS;
>>>
>>> -    if (!is_net) {
>>> -        /* Add leading / to fs paths, because they're absolute */
>>> -        snprintf(filename, sizeof(filename), "/%s", path);
>>> -    } else {
>>> -        snprintf(filename, sizeof(filename), "%s", path);
>>> -    }
>>> +    snprintf(filename, sizeof(filename), "%s", path);
>>
>> I'm pretty sure that code was there for a reason. Without, grub would not find its config file iirc.
>>
>> Alex
> 
> Alex, you introduced the logic in 492716662fbdc08e254dda2c209b320e2bf6c837.
> 
> Typically the grubaa64.efi file is on an EFI partition while the rest of
> GRUB is on the boot partition which could be ext2. In which way would it
> make a difference to GRUB whether the path to grubaa64.efi is absolute
> or relative? The next thing it will load is anyway on a different path.
> 
> On my Odroid C2 I built U-Boot with the patch and executed
> 
> => load mmc 0:1 ${fdt_addr_r} dtb
> 22925 bytes read in 5 ms (4.4 MiB/s)
> => load mmc 0:4 ${kernel_addr_r} EFI/debian/grubaa64.efi
> 139264 bytes read in 8 ms (16.6 MiB/s)
> => bootefi ${kernel_addr_r} ${fdt_addr_r}
> 
> And the system booted normally. grub/ is on mmc 0:1 so on a different
> partition of the same deive.
> 
> In grubaa64.efi I found a string (,msdos1)/grub. Wouldn't this be the
> path GRUB is using on the boot device?
> 
> Can you provide any evidence that GRUB is actually using anything else
> than the device part of the device path?

I can no longer reproduce the problem that York reported. So yes, I'm ok
with simplifying the logic. But beware that there *might* be something
lurking we just don't remember and that eventually there could be a bug
report.


Alex