On 4/10/19 8:48 AM, Igor Opaniuk wrote:
Hi Heinrich,
Thanks for looking into this,
On Tue, Apr 9, 2019 at 11:28 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
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@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)
That's why I added RFC tag, as I definitely don't understand the full picture of what's going here(and lack of time to discover) :)
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
Thanks for the explanation.
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.
So with the changes:
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 97b4356167..ef707e2505 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -821,6 +821,8 @@ struct efi_device_path *efi_dp_from_file(struct blk_desc *desc, int part, fpsize = sizeof(struct efi_device_path) + 2 * (strlen(path) + 1); dpsize += fpsize;
printf("dpsize %zu\n", dpsize);start = buf = dp_alloc(dpsize + sizeof(END)); if (!buf) return NULL;@@ -837,6 +839,8 @@ struct efi_device_path *efi_dp_from_file(struct blk_desc *desc, int part, buf += fpsize;
*((struct efi_device_path *)buf) = END;
printf("desc %p\n", desc);printf("dp length %zu\n", efi_dp_size(start)); return start;}
Output: dpsize 153 desc 2daa3d30
And U-boot hangs on efi_dp_size(start) invocation, so I never receive any output from the last printf() invocation.
In http://git.denx.de/?p=u-boot-efi.git;a=shortlog;h=refs/heads/efi-2019-07 I have already added a patch the problem with efi_dp_size().
efi_loader: move efi_save_gd() call to board_r.c http://git.denx.de/?p=u-boot-efi.git;a=commit;h=5658c83d1c3637d4aa5bc366ab98...
So I suggest that you start from the efi-2019-07 branch.
Regards
Heinrich
Please, indicate the config file that you are using.
colibri_imx6_defconfig
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;
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot