Skip to content

mkimage: Tidy up error handling

Stefano Babic u-boot 1 version New

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest0 of 14 patches 0 repliesNew

Patches in v1 No applies recorded yet

None of the 14 patches the cover letter announces are in this archive.

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

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

Hi Tom, Simon,

Quoted 81 lines · Tom Rini
Quoted 80 lines · Simon Glass

Hi,

Quoted 66 lines · Tom Rini
Quoted 65 lines · Stefano Babic

Hi Simon,

Quoted 5 lines · Simon Glass

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:

tools/Makefile +2 -2
index 63355aa..f72294a 100644
@@ -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)
Quoted 7 lines
Quoted 4 lines
tools/Makefile +0 -0
 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.

That patch seems reasonable to me for now. My series is intended for the next release.

Quoted 9 lines
Quoted 1 line

Perhaps verify_header() should return a value meaning 'possibly'?

So, this was broken by:
commit 0ca6691c2ec20ff264d882854c339795579991f5

Author: Guilherme Maciel Ferreira <[email protected]>
Date:   Thu Jan 15 02:48:05 2015 -0200

imagetool: move common code to imagetool module

Yes, I had bisected myself and I get this commit - but the patch is not the readon of the breakage.

Quoted 1 line

And yes, perhaps some way to say to say "maybe" is enough.

Right, but there is a side effect. Even if a "maybe" is returned (or let's say, a "I can't check"), the effect is that "GP Header: Size XXXX" is still printed, because for the gpimage this *could* be a correct image.

Quoted 3 lines

Or maybe we
just need to allow some to say that they can't verify? I think we're thinking along the same lines.

Regards,
Stefano

Signature · 5 lines
=====================================================================
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
=====================================================================

Read the cover letter in its thread