From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jonas Gorski <jogo@openwrt.org>
Date: Fri, 3 May 2013 18:31:28 +0200
Subject: [U-Boot] [PATCH] tools: default image: use ih_size for checking
 data size
In-Reply-To: <20130503155425.D4503380E09@gemini.denx.de>
References: <1367581025-26740-1-git-send-email-jogo@openwrt.org>
	<20130503150444.0F83D38044A@gemini.denx.de>
	<20130503174205.00000070@unknown>
	<20130503155425.D4503380E09@gemini.denx.de>
Message-ID: <20130503183128.00000900@unknown>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Fri, 03 May 2013 17:54:25 +0200
Wolfgang Denk <wd@denx.de> wrote:

> Dear Jonas Gorski,
> 
> In message <20130503174205.00000070@unknown> you wrote:
> >
> > > Sorry, I don't know how you create your image files, but you must
> > > be doing something fundamentally wrong.  If mkimage reports a bug
> > > here, it is probably right.  If the actual payload size is
> > > different from the content of the  ih_size  field, then your
> > > image _is_ broken.
> > 
> > That what else for is the ih_size field then except to say what the
> > actual datasize is? mkuimage also sets this fields to the
> > correct size.
> 
> It's exactly for this very purpose, and allows for consistency
> checking.  You run into errors, becuase your images are not correct.
> For plain legacy image format (i. e. we are not talking about
> multi-file image format here), header size (= 64 bytes) plus the
> content of the ih_size  field will give the total file size of the
> image.  If this condition is not met, then your image is broken.
> 
> > And this isn't from me, but this is how most firmware images are
> 
> Define "most".

Most images I have seen for kirkwood, ar71xx, ralink, and I'm sure
there are more.

> > created for devices using U-Boot, i.e. uImage packed kernel +
> > appended rootfs. Also U-Boot itself only cares for the first
> > ih_size bytes of the image and not for any "garbage" that might be
> > behind it:
> 
> Yes, because in U-Boot we have no notion of "files" and thus no
> indication where an image ends.  Otherwise such a check would be
> there.
> 
> > It checks the crc of the first ih_size bytes after the image_header
> > - and my change changes mkimage to mirror that behaviour.
> 
> But this is wrong.

But that's what U-Boot does. The quoted code is taken straight from
git. My impression was that if U-Boot accepts this image, then mkimage
should, too.

> > It still reports data errors if the checksum is wrong for the data
> > actually specified by the image header, but now it actually respects
> > the length of the data field.
> 
> Let me repeat: a valid image will have sizeof(struct image_header)
> plus ih_size == file size.  If this condition is not true, then your
> image is broken.

Okay, after a bit of IRC discussion the following compromise:

If the header is correct, and the crc is correct for the ih_size data,
then assume everything is fine, but print out a warning if the file
length does not match the header lengh + ih_size.

My typical use case would be looking at vendor images or flashdumps and
check if they have a valid uimage header. With this patch I can do
it easily with the imagefile/flashdump, without it I would need to parse
the header first manually, find the ih_size field, truncate the file to
the expected size, and then hope the header is valid.


Best regards,
Jonas Gorski