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

[Stefano Babic]

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.

Best regards,
Stefano

> This series tidies this up a little, to make mkimage more friendly.
> 
> 
> Simon Glass (14):
>   mkimage: Honour the default image type with auto-fit
>   mkimage: Explain the auto-fit imagefile special case
>   mkimage: Require a data file when auto-fit is used
>   mkimage: Drop premature setting of params.fit_image_type
>   mkimage: Drop blank line before main()
>   image: Correct auto-fit architecture property name
>   image: Convert the IH_... values to enums
>   image: Create a table of information for each category
>   image: Add a name for invalid types
>   image: Add functions to obtain category information
>   mkimage: Allow display of a list of any image header category
>   mkimage: Use generic code for showing an 'image type' error
>   mkimage: Show item lists for all categories
>   tools: Allow building with debug enabled
> 
>  Kconfig           |   9 +++
>  Makefile          |   3 +-
>  common/image.c    |  87 ++++++++++++++++++++-
>  include/image.h   | 230 ++++++++++++++++++++++++++++++++++--------------------
>  tools/fit_image.c |   3 +-
>  tools/mkimage.c   |  69 +++++++++-------
>  6 files changed, 280 insertions(+), 121 deletions(-)
> 


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================