efi_loader: Exit in efi_set_bootdev() upon invalid "desc"

PATCH Applied Stefan Roese 3 participants 5 messages Nov 17, 2017 – Dec 4, 2017

lore

Share & tools

lore

Stefan Roese

author

2017-11-17 07:47 UTC #0 lore raw

PATCH 1/1 efi_loader: Exit in efi_set_bootdev() upon invalid "desc"

When trying to load an image from a non-existent USB key, U-Boot v2017.11 crashes on my x86 platform:

=> load usb 0:1 03000000 abc
General Protection
EIP: 0010:[<7b59030d>] EFLAGS: 00010286
Original EIP :[<fff4330d>]
...

This used to work in v2017.09. Testing has shown, that this bug was introduced with patch 95c5553e [efi_loader: refactor boot device and loaded_image handling].

This patch now checks if a valid "desc" is returned from blk_get_dev() and only continues when "desc" is available. Resulting in this cmd output (again):

=> load usb 0:1 03000000 abc
** Bad device usb 0 **

Signed-off-by Stefan Roese <[email protected]>

Cc Rob Clark <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Alexander Graf <[email protected]>

Cc Marek Vasut <[email protected]>

Cc Bin Meng <[email protected]>

---
 cmd/bootefi.c | 2 ++
 1 file changed, 2 insertions(+)

cmd/bootefi.c +2 -0

index 478bc116e2..b468a20d82 100644
@@ -390,6 +390,8 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
 		int part;
 
 		desc = blk_get_dev(dev, simple_strtol(devnr, NULL, 10));
+		if (!desc)
+			return;
 		part = parse_partnum(devnr);
 
 		bootefi_device_path = efi_dp_from_part(desc, part);

Signature · 1 line

2.15.0

Heinrich Schuchardt

2017-11-17 17:26 UTC #1 lore raw

PATCH 1/1 efi_loader: Exit in efi_set_bootdev() upon invalid "desc"

Quoted 36 lines · Stefan Roese

When trying to load an image from a non-existent USB key, U-Boot v2017.11 crashes on my x86 platform:
=> load usb 0:1 03000000 abc
General Protection
EIP: 0010:[<7b59030d>] EFLAGS: 00010286
Original EIP :[<fff4330d>]
...
This used to work in v2017.09. Testing has shown, that this bug was introduced with patch 95c5553e [efi_loader: refactor boot device and loaded_image handling].
This patch now checks if a valid "desc" is returned from blk_get_dev() and only continues when "desc" is available. Resulting in this cmd output (again):
=> load usb 0:1 03000000 abc
** Bad device usb 0 **
Signed-off-by Stefan Roese <[email protected]>
Cc Rob Clark <[email protected]>
Cc Heinrich Schuchardt <[email protected]>
Cc Alexander Graf <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Bin Meng <[email protected]>
---
  cmd/bootefi.c | 2 ++
  1 file changed, 2 insertions(+)
cmd/bootefi.c +0 -0
index 478bc116e2..b468a20d82 100644

cmd/bootefi.c void efi_set_bootdev(const char *dev, const char *devnr, const char *path) · line 397

Quoted 8 lines

cmd/bootefi.c +2 -0

@@ -390,6 +390,8 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
  		int part;
 
  		desc = blk_get_dev(dev, simple_strtol(devnr, NULL, 10));
+		if (!desc)
+			return;
  		part = parse_partnum(devnr);
 
  		bootefi_device_path = efi_dp_from_part(desc, part);

blk_get_dev can return NULL. We should abort efi_set_bootdev() when receiving this value.

But I could not reproduce your problem on qemu-x86_64.

Reviewed-by Heinrich Schuchardt <[email protected]>

Alexander Graf

Applied

2017-12-04 08:59 UTC #2 lore raw

In reply to Stefan Roese

Quoted 20 lines

When trying to load an image from a non-existent USB key, U-Boot v2017.11 crashes on my x86 platform:
=> load usb 0:1 03000000 abc
General Protection
EIP: 0010:[<7b59030d>] EFLAGS: 00010286
Original EIP :[<fff4330d>]
...
This used to work in v2017.09. Testing has shown, that this bug was introduced with patch 95c5553e [efi_loader: refactor boot device and loaded_image handling].
This patch now checks if a valid "desc" is returned from blk_get_dev() and only continues when "desc" is available. Resulting in this cmd output (again):
=> load usb 0:1 03000000 abc
** Bad device usb 0 **

Quoted 7 lines

Signed-off-by Stefan Roese <[email protected]>
Cc Rob Clark <[email protected]>
Cc Heinrich Schuchardt <[email protected]>
Cc Alexander Graf <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Bin Meng <[email protected]>
Reviewed-by Heinrich Schuchardt <[email protected]>

Thanks, applied to efi-next

Alex

Stefan Roese

author

2017-12-04 11:52 UTC #3 lore raw

Hi Alex,

Quoted 28 lines · Alexander Graf

Quoted 20 lines
When trying to load an image from a non-existent USB key, U-Boot v2017.11 crashes on my x86 platform:
=> load usb 0:1 03000000 abc
General Protection
EIP: 0010:[<7b59030d>] EFLAGS: 00010286
Original EIP :[<fff4330d>]
...
This used to work in v2017.09. Testing has shown, that this bug was introduced with patch 95c5553e [efi_loader: refactor boot device and loaded_image handling].
This patch now checks if a valid "desc" is returned from blk_get_dev() and only continues when "desc" is available. Resulting in this cmd output (again):
=> load usb 0:1 03000000 abc
** Bad device usb 0 **
Quoted 7 lines
Signed-off-by Stefan Roese <[email protected]>
Cc Rob Clark <[email protected]>
Cc Heinrich Schuchardt <[email protected]>
Cc Alexander Graf <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Bin Meng <[email protected]>
Reviewed-by Heinrich Schuchardt <[email protected]>

Quoted 1 line

Thanks, applied to efi-next

Thanks. Does "efi-next" mean this patch will not go into v2018.01? As it fixes a regression, I would really like to see it included in the release.

Thanks,
Stefan

Alexander Graf

2017-12-04 12:08 UTC #4 lore raw

Quoted 34 lines

Am 04.12.2017 um 12:52 schrieb Stefan Roese <[email protected]>:
Hi Alex,
Quoted 27 lines · Alexander Graf
Quoted 27 lines
When trying to load an image from a non-existent USB key, U-Boot v2017.11 crashes on my x86 platform:
=> load usb 0:1 03000000 abc
General Protection
EIP: 0010:[<7b59030d>] EFLAGS: 00010286
Original EIP :[<fff4330d>]
...
This used to work in v2017.09. Testing has shown, that this bug was introduced with patch 95c5553e [efi_loader: refactor boot device and loaded_image handling].
This patch now checks if a valid "desc" is returned from blk_get_dev() and only continues when "desc" is available. Resulting in this cmd output (again):
=> load usb 0:1 03000000 abc
** Bad device usb 0 **
Signed-off-by Stefan Roese <[email protected]>
Cc Rob Clark <[email protected]>
Cc Heinrich Schuchardt <[email protected]>
Cc Alexander Graf <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Bin Meng <[email protected]>
Reviewed-by Heinrich Schuchardt <[email protected]>

Quoted 3 lines

Quoted 1 line
Thanks, applied to efi-next
Thanks. Does "efi-next" mean this patch will not go into v2018.01?

Yes, sorry for the confusion :)

Alex

Quoted 5 lines

As it fixes a regression, I would really like to see it included in the release.
Thanks,
Stefan