All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.