[U-Boot] [PATCH v2] tools/mkimage: ignore trailing garbage

Ignore trailing data after the uimage data and only complain if the file is too small.
(E.G. if the uImage was stored in flash and hence padded to the flash sector size).
Signed-off-by: Peter Korsgaard jacmet@sunsite.dk --- tools/mkimage.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
Changes since v1: - Fix a compiler warning on 64bit
diff --git a/tools/mkimage.c b/tools/mkimage.c index 967fe9a..b19cd6f 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -523,7 +523,14 @@ image_verify_header (char *ptr, int image_size) }
data = ptr + sizeof(image_header_t); - len = image_size - sizeof(image_header_t) ; + len = ntohl(hdr->ih_size); + if (len > (image_size - sizeof(image_header_t))) { + fprintf (stderr, + "%s: ERROR: "%s" is too short (%d vs %d bytes)!\n", + cmdname, imagefile, + image_size - (int)sizeof(image_header_t), len); + exit (EXIT_FAILURE); + }
if (crc32 (0, data, len) != ntohl(hdr->ih_dcrc)) { fprintf (stderr,

Use lseek rather than fstat for file size for list mode, so mkimage -l /dev/mtdblockN works (stat returns st_size == 0 for devices).
Notice that you have to use /dev/mtdblockN and not /dev/mtdN, as the latter doesn't support mmap.
Signed-off-by: Peter Korsgaard jacmet@sunsite.dk --- tools/mkimage.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)
Changes since v1: - Incorporate remarks from Thomas/Wolfgang (update error message, don't use struct stat) diff --git a/tools/mkimage.c b/tools/mkimage.c index b19cd6f..1d4619d 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -202,23 +202,25 @@ NXTARG: ; }
if (lflag) { + off_t size; /* * list header information of existing image */ - if (fstat(ifd, &sbuf) < 0) { - fprintf (stderr, "%s: Can't stat %s: %s\n", + size = lseek(ifd, 0, SEEK_END); + if (size == (off_t)-1) { + fprintf (stderr, "%s: Can't get size of %s: %s\n", cmdname, imagefile, strerror(errno)); exit (EXIT_FAILURE); }
- if ((unsigned)sbuf.st_size < image_get_header_size ()) { + if ((unsigned)size < image_get_header_size ()) { fprintf (stderr, "%s: Bad size: "%s" is no valid image\n", cmdname, imagefile); exit (EXIT_FAILURE); }
- ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, ifd, 0); + ptr = mmap(0, size, PROT_READ, MAP_SHARED, ifd, 0); if (ptr == MAP_FAILED) { fprintf (stderr, "%s: Can't read %s: %s\n", cmdname, imagefile, strerror(errno)); @@ -227,14 +229,14 @@ NXTARG: ;
if (fdt_check_header (ptr)) { /* old-style image */ - image_verify_header ((char *)ptr, sbuf.st_size); + image_verify_header ((char *)ptr, size); image_print_contents ((image_header_t *)ptr); } else { /* FIT image */ fit_print_contents (ptr); }
- (void) munmap((void *)ptr, sbuf.st_size); + (void) munmap((void *)ptr, size); (void) close (ifd);
exit (EXIT_SUCCESS);

Dear Peter Korsgaard,
In message 1228304125-6669-2-git-send-email-jacmet@sunsite.dk you wrote:
Use lseek rather than fstat for file size for list mode, so mkimage -l /dev/mtdblockN works (stat returns st_size == 0 for devices).
Notice that you have to use /dev/mtdblockN and not /dev/mtdN, as the latter doesn't support mmap.
Signed-off-by: Peter Korsgaard jacmet@sunsite.dk
As mentioned before, this doesn't work because the length you are checking is not the image length, but the device size.
Best regards,
Wolfgang Denk

"Wolfgang" == Wolfgang Denk wd@denx.de writes:
Hi,
Wolfgang> As mentioned before, this doesn't work because the length you are Wolfgang> checking is not the image length, but the device size.
Yes, because you rejected the patch to fix this.
So, what do you suggest?

Dear Peter Korsgaard,
In message 1228304125-6669-1-git-send-email-jacmet@sunsite.dk you wrote:
Ignore trailing data after the uimage data and only complain if the file is too small.
(E.G. if the uImage was stored in flash and hence padded to the flash sector size).
I reject this patch. The length checking of the image is intentional and shall not be removed.
Best regards,
Wolfgang Denk
participants (2)
-
Peter Korsgaard
-
Wolfgang Denk