Thanks Heinrich! Will test it ASAP.
On Wed, Apr 10, 2019 at 8:36 PM Heinrich Schuchardt <[email protected]> wrote: > > 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 <[email protected]> > > 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 <[email protected]> > >>> --- > >>> 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=5658c83d1c3637d4aa5bc366ab987b776ea105eb > > 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 > >> [email protected] > >> https://lists.denx.de/listinfo/u-boot > > > > > > > > _______________________________________________ > U-Boot mailing list > [email protected] > https://lists.denx.de/listinfo/u-boot -- Best regards - Freundliche Grüsse - Meilleures salutations Senior Development Engineer, Igor Opaniuk Toradex AG Altsagenstrasse 5 | 6048 Horw/Luzern | Switzerland | T: +41 41 500 48 00 (main line) _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
