[U-Boot] [PATCH] tools: default image: use ih_size for checking data size

[Jonas Gorski <jogo@openwrt.org>]

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

> Dear Jonas Gorski,
> 
> In message <1367581025-26740-1-git-send-email-jogo@openwrt.org> you
> wrote:
> > Common image usage is uImage + appended rootfs, so the the uImage
> > data
> 
> No, this is not at all "common usage".  Actually this something you
> should never do.
> 
> > is only part of the total image. So read out and use the header's
> > ih_size field instead of the total file size.
> > 
> > To prevent reading over the end of the buffer, check that the image
> > file is big enough to contain the data before calculating its
> > checksum.
> > 
> > Before:
> > ~# mkimage -l dir665_fw_100NA.bin
> > mkimage: ERROR: "dir665_fw_100NA/dir665_fw_100NA.bin" has corrupted
> > data!
> 
> 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.

And this isn't from me, but this is how most firmware images are
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:

int image_check_dcrc(const image_header_t *hdr)
{
	...
	ulong len = image_get_data_size(hdr);
	...
	ulong dcrc = crc32_wd(...,len,...);
	...
}

static inline uint32_t image_get_data_size(const image_header_t *hdr)
{
	return image_get_size(hdr); /* == hdr->ih_size */
}

It checks the crc of the first ih_size bytes after the image_header -
and my change changes mkimage to mirror that behaviour.
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.

One might even argue that it is now *more* correct, because I can
can easily construct you an image that will pass the "old" mkimage, but
will be rejected by U-Boot, by creating a different length payload
that has the same crc32 as expected by the header.


Best regards
Jonas Gorski