Hi Heinrich
> From: Heinrich Schuchardt <xypron.g...@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.delau...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
