Hi Heinrich
From: Heinrich Schuchardt xypron.glpk@gmx.de
On 4/10/19 11:02 AM, Patrick Delaunay wrote:
Check the value of block_dev before to use this pointer.
This patch solves problem for the command "load" when ubifs is previously mounted: in this case the function blk_get_device_part_str("ubi 0") don't return error but return block_dev = NULL and then data abort.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
To reproduce the issue, I have a boot script 'boot.scr.uimg' with a load command executed during ubi boot:
load ${devtype} ${devnum}:${distro_bootpart} ${m4fw_addr} ${m4fw_name}
I have a data abort for call stack:
- do_load_wrapper for "ubi 0"
-- efi_set_bootdev --- efi_dp_from_name
=> desc = 0 and data abort for access to 'desc->*'
Thanks for reporting and analyzing the problem
Where exactly is the NULL dereference occurring?
I see the issue on v2018.11 and I adapt the patch for v2019.04 (as efi_dp_from_name as be created in v2019.01)
On v2018.11, the stack frame was
do_load_wrapper => efi_set_bootdev ==> efi_dp_from_name ===> efi_dp_from_part ====> dp_part_size
With result for blk_get_device_part_str dev="ubi" devnr="0:" path="/boot.scr.uimg"
desc =00000000 part=0
And the crash occurs in the function dp_part_size / called from efi_dp_from_part
In trace : pc : [<ffc91d28>] lr : [<ffc92383>] reloc pc : [<c0166d28>] lr : [<c0167383>]
with symbol
System.map:c0167358 T efi_dp_is_multi_instance System.map:c0167378 T efi_dp_from_part System.map:c01673a4 T efi_dp_part_node System.map:c01673c8 T efi_dp_from_file
System.map:c0166d22 t dp_part_size System.map:c0166d50 t dp_part_node
Igor reported a similar bug for a USB device in cmd: fs: fix data abort in load cmd https://lists.denx.de/pipermail/u-boot/2019-April/364484.htmll
I also proposed a protection for the same issue in ums command http://patchwork.ozlabs.org/project/uboot/list/?series=68096
lib/efi_loader/efi_device_path.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 53b40c8..fd57be8 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -970,7 +970,7 @@ efi_status_t efi_dp_from_name(const char *dev, const
char *devnr,
if (!is_net) { part = blk_get_device_part_str(dev, devnr, &desc, &fs_partition, 1);
if (part < 0)
if (part < 0 || !desc)part = 0, desc = NULL occurs for UBI if the UBI file system is mounted.
Returning an error here means in the end that we will not be able to install run GRUB from the UBI device because we cannot describe the boot device.
I think that UBI volumes should be handled like any other block device. This will avoid having separate program paths for UBI and not UBI.
Heiko and Kyungmin could you, please, explain why UBI currently is not providing a struct blk_desc * block descriptor and how this can be fixed.
Best regards
Heinrich
return EFI_INVALID_PARAMETER; if (device)
Best regards
Patrick