[U-Boot] [PATCH 2/3] efi: add protection for block_dev

[Heinrich Schuchardt]

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 at 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?

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