[U-Boot] [PATCH 1/1] efi_loader: do not add \ in efi_fs_from_name()
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@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); /* DOS style file path: */ s = filename; while ((s = strchr(s, '/')))
Am 23.02.2019 um 02:20 schrieb Heinrich Schuchardt xypron.glpk@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@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
/* DOS style file path: */ s = filename; while ((s = strchr(s, '/'))) -- 2.20.1
On 2/23/19 2:37 PM, Alexander Graf wrote:
Am 23.02.2019 um 02:20 schrieb Heinrich Schuchardt xypron.glpk@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@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?
Best regards
Heinrich
/* DOS style file path: */ s = filename; while ((s = strchr(s, '/'))) -- 2.20.1
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@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@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
participants (2)
-
Alexander Graf -
Heinrich Schuchardt