public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stefano Babic <sbabic@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 00/14] mkimage: Tidy up error handling
Date: Fri, 1 Jul 2016 10:44:04 +0200	[thread overview]
Message-ID: <57762D54.3030302@denx.de> (raw)
In-Reply-To: <1467305540-13607-1-git-send-email-sjg@chromium.org>

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
=====================================================================

  parent reply	other threads:[~2016-07-01  8:44 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-30 16:52 [U-Boot] [PATCH 00/14] mkimage: Tidy up error handling Simon Glass
2016-06-30 16:52 ` [U-Boot] [PATCH 01/14] mkimage: Honour the default image type with auto-fit Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:48   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 02/14] mkimage: Explain the auto-fit imagefile special case Simon Glass
2016-07-06 15:08   ` Joe Hershberger
2016-07-16 13:48   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 03/14] mkimage: Require a data file when auto-fit is used Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:48   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 04/14] mkimage: Drop premature setting of params.fit_image_type Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:48   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 05/14] mkimage: Drop blank line before main() Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:48   ` [U-Boot] [U-Boot,05/14] " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 06/14] image: Correct auto-fit architecture property name Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:48   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 07/14] image: Convert the IH_... values to enums Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:48   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 08/14] image: Create a table of information for each category Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:48   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 09/14] image: Add a name for invalid types Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:48   ` [U-Boot] [U-Boot,09/14] " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 10/14] image: Add functions to obtain category information Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:49   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 11/14] mkimage: Allow display of a list of any image header category Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:49   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 12/14] mkimage: Use generic code for showing an 'image type' error Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:49   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 13/14] mkimage: Show item lists for all categories Simon Glass
2016-07-02  1:36   ` Tom Rini
2016-07-16 13:49   ` [U-Boot] [U-Boot, " Tom Rini
2016-06-30 16:52 ` [U-Boot] [PATCH 14/14] tools: Allow building with debug enabled Simon Glass
2016-07-02  1:37   ` Tom Rini
2016-07-16 13:49   ` [U-Boot] [U-Boot, " Tom Rini
2016-07-01  8:44 ` Stefano Babic [this message]
2016-07-01 15:48   ` [U-Boot] [PATCH 00/14] mkimage: Tidy up error handling Tom Rini
2016-07-01 16:15     ` Simon Glass
2016-07-01 16:45       ` Tom Rini
2016-07-01 18:33         ` Stefano Babic
2016-07-01 18:38           ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57762D54.3030302@denx.de \
    --to=sbabic@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox