[U-Boot] [RFC 1/1] cmd: fs: fix data abort in load cmd

[Heinrich Schuchardt]

On 4/9/19 3:08 PM, Igor Opaniuk wrote:
> With CONFIG_CMD_BOOTEFI=y, load command causes data abort
> when path_to_uefi(fp->str, path) tries to write uefi path out of
> bounds of u16 str[] array (check efi_device_path_file_path struct for
> details). This is caused by unproper handling of void *buf pointer
> in efi_dp_from_file(), particularly when the buf pointer value is changed
> after dp_part_fill() invocation.
>
>> load usb 0:1 0x12000000 imx6dl-colibri-eval-v3.dtb
> pc : [<2fab48ae>]	   lr : [<2fab4339>]
> reloc pc : [<178338ae>]	   lr : [<17833339>]
> sp : 2da77120  ip : 00000003	 fp : 00000005
> r10: 2daa31d0  r9 : 2da80ea8	 r8 : 00000001
> r7 : 2daa3098  r6 : 2ca75040	 r5 : 2da77148  r4 : 0000003a
> r3 : 00000069  r2 : 2ca750a3	 r1 : 2daa3104  r0 : 2ca7509f
> Flags: nzCv  IRQs off  FIQs off  Mode SVC_32
> Code: 4630fb31 81f0e8bd e7d84606 bf082b2f (f822235c)
> Resetting CPU ...
>

Thanks for reporting the problem.

> With the change suggested:
>
>> load usb 0:1 0x12000000 imx6dl-colibri-eval-v3.dtb
> 5675440 bytes read in 188 ms (28.8 MiB/s)
>
> Signed-off-by: Igor Opaniuk <igor.opaniuk at toradex.com>
> ---
>  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 53b40c8c3c..97b4356167 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -829,7 +829,7 @@ struct efi_device_path *efi_dp_from_file(struct blk_desc *desc, int part,
>  		buf = dp_part_fill(buf, desc, part);
>
>  	/* add file-path: */
> -	fp = buf;
> +	fp = start;

This cannot be correct. dp_part_fill() is meant to set buf to the end of
the partition device path, e.g.
/VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)/HD(1,MBR,0xd1535d21,0x1,0x7f)

In the lines below we want to add a further device path node with the
filename followed by the end node, e.g.

/VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)/HD(1,MBR,0xd1535d21,0x1,0x7f)/Shell.efi

With your patch we end up with a device path containing only the file
name and the end node, e.g.

/Shell.efi

If you think this is an out of bound problem we must fix the estimation
of the device path size.

For better understanding the problem could you, please, print the value
of dpsize and then call dp_alloc() with a sufficiently large argument.

Before the return statement add

printf("desc %p\n", desc);
printf("dp length %zu\n", efi_dp_size(start));

This should provide the calculated device path length and its actual size.

Please, indicate the config file that you are using.

Best regards

Heinrich

>  	fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
>  	fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
>  	fp->dp.length = fpsize;
>