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

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

[Peter Korsgaard]

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,
-- 
1.5.6.5

[U-Boot] [PATCH v2] tools/mkimage: use lseek rather than fstat for file size for -l option

[Peter Korsgaard]

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);
-- 
1.5.6.5

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

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Quantum particles: The dreams that stuff is made of.

[U-Boot] [PATCH v2] tools/mkimage: use lseek rather than fstat for file size for -l option

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Eeeek!
'eval' on strings should have been named 'evil'.    -- Tom Phoenix in
        <Pine.GSO.3.96.980526121813.27437N-100000@user2.teleport.com>

[U-Boot] [PATCH v2] tools/mkimage: use lseek rather than fstat for file size for -l option

[Peter Korsgaard]

>>>>> "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?

-- 
Bye, Peter Korsgaard