Skip to contentSkip to conversation

tools/mkimage: ignore trailing garbage

PATCH New Peter Korsgaard 2 participants 5 messages Dec 3, 2008

lore
Share & tools
lore

Peter Korsgaard

author
#0 lore raw

PATCH v2 1/1 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 <[email protected]>
---
 tools/mkimage.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

Changes since v1:
- Fix a compiler warning on 64bit

tools/mkimage.c +8 -1
index 967fe9a..b19cd6f 100644
@@ -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,
Signature · 1 line
1.5.6.5
author
#1 lore raw

PATCH v2 1/1 tools/mkimage: use lseek rather than fstat for file size for -l option

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 <[email protected]>
---
 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)

tools/mkimage.c +8 -6
index b19cd6f..1d4619d 100644
@@ -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);
Signature · 1 line
1.5.6.5
#2 lore raw

PATCH v2 1/1 tools/mkimage: use lseek rather than fstat for file size for -l option

Dear Peter Korsgaard,

Quoted 7 lines · In message <[email protected]> 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 <[email protected]>

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

Signature · 6 lines
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
        <[email protected]>
author
#3 lore raw

PATCH v2 1/1 tools/mkimage: use lseek rather than fstat for file size for -l option

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Wolfgang" == Wolfgang Denk <[email protected]> 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?

Signature · 1 line
Bye, Peter Korsgaard
#4 lore raw

PATCH v2 1/1 tools/mkimage: ignore trailing garbage

In reply to Peter Korsgaard

Dear Peter Korsgaard,

Quoted 5 lines · In message <[email protected]> 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

Signature · 4 lines
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.