[U-Boot] [PATCH 00/14] mkimage: Tidy up error handling

[Tom Rini]

On Fri, Jul 01, 2016 at 10:44:04AM +0200, Stefano Babic wrote:
> Hi Simon,
> 
> On 30/06/2016 18:52, Simon Glass wrote:
> > There are a few problems when mkimage is provided with invalid arguments.
> > In one case it crashes. When an invalid image type it is provided it lists
> > the valid types, but this is not implemented for compression, architecture
> > or OS.
> > 
> 
> There is another issue related to mkimage. It looks like it is broken
> since a lot of time, but it appears it was not noted.
> 
> mkimage -l is broken. Testing with i.MX images (imximage), it does not
> show the header, but it reports the output as it as a "gpimage".
> 
> In fact:
> 
> ./tools/mkimage -l test.imx
> GP Header: Size d1002040 LoadAddr 8017
> 
> It should be:
> 
> ./tools/mkimage -l test.imx
> Image Type:   Freescale IMX Boot Image
> Image Ver:    2 (i.MX53/6/7 compatible)
> Data Size:    331776 Bytes = 324.00 kB = 0.32 MB
> Load Address: 177ff420
> Entry Point:  17800000
> 
> The reason is due to the format of the gpimage itself. It has no magic
> number, and checking for its correctness means in gph_verify_header()
> just check that size and address are not NULL. That let think that the
> image is a gpimage, it is not. gpimage simply uses the image and does
> not let to check for further image headers that are put int the (sorted)
> list with the U_BOOT_IMAGE_TYPE. Image types that are in the list
> *before* gpimage are correctly recognized and printed, the following (as
> imximage) not.
> 
> A quick fix (maybe just for the release ?) should be to let gpimage (but
> I do not know if there is another image type that misbehaves as it does)
> as the last one in the list: if all detections fail, the last one
> without any possibility for detection (gpimage) runs. The following
> patch works for me:
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 63355aa..f72294a 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -76,8 +76,6 @@ dumpimage-mkimage-objs := aisimage.o \
>  			lib/fdtdec.o \
>  			fit_common.o \
>  			fit_image.o \
> -			gpimage.o \
> -			gpimage-common.o \
>  			common/image-fit.o \
>  			image-host.o \
>  			common/image.o \
> @@ -100,6 +98,8 @@ dumpimage-mkimage-objs := aisimage.o \
>  			zynqimage.o \
>  			zynqmpimage.o \
>  			$(LIBFDT_OBJS) \
> +			gpimage.o \
> +			gpimage-common.o \
>  			$(RSA_OBJS-y)
> 
>  dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
> 
> What do you think ? If this could be a solution for release, I send a
> formal patch.

Ug, I think we need what you're saying at least for release.  I'm
kicking off a big bisect now to see just when this last worked right.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160701/69a97215/attachment.sig>