Skip to contentSkip to conversation

tools/mkimage: ignore trailing garbage

PATCH Superseded Peter Korsgaard 3 participants 8 messages Dec 1, 2008 – Dec 3, 2008

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 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).

---
 tools/mkimage.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
tools/mkimage.c +8 -1
index 967fe9a..58fd20f 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 - 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
#1 lore raw

PATCH 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.

---
 tools/mkimage.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
tools/mkimage.c +2 -1
index 58fd20f..ca8254f 100644
@@ -205,7 +205,8 @@ NXTARG:		;
 		/*
 		 * list header information of existing image
 		 */
-		if (fstat(ifd, &sbuf) < 0) {
+		sbuf.st_size = lseek(ifd, 0, SEEK_END);
+		if (sbuf.st_size == (off_t)-1) {
 			fprintf (stderr, "%s: Can't stat %s: %s\n",
 				cmdname, imagefile, strerror(errno));
 			exit (EXIT_FAILURE);
Signature · 1 line
1.5.6.5
#2 lore raw

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

Hi Peter,

Quoted 16 lines · 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.

---
 tools/mkimage.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
tools/mkimage.c +0 -0
index 58fd20f..ca8254f 100644

tools/mkimage.c NXTARG: ; · line 211

Quoted 8 lines
tools/mkimage.c +2 -1
@@ -205,7 +205,8 @@ NXTARG:             ;
               /*
                * list header information of existing image
                */
-               if (fstat(ifd, &sbuf) < 0) {
+               sbuf.st_size = lseek(ifd, 0, SEEK_END);
+               if (sbuf.st_size == (off_t)-1) {
                       fprintf (stderr, "%s: Can't stat %s: %s\n",
                               cmdname, imagefile, strerror(errno));

I'd change the error message as well, to be independent of the tool used to get the file size. For example:
fprintf (stderr, "%s: Can't get size of %s: %s\n",

Best regards,
Thomas

#3 lore raw

PATCH 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

"Thomas" == Thomas De Schampheleire <[email protected]> writes:

Thomas> I'd change the error message as well, to be independent of the tool Thomas> used to get the file size. For example:
Thomas> fprintf (stderr, "%s: Can't get size of %s: %s\n",

Ahh yes, good idea.

#4 lore raw

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

Dear Peter Korsgaard,

Quoted 13 lines · In message <[email protected]> you wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Thomas" == Thomas De Schampheleire <[email protected]> writes:

Thomas> I'd change the error message as well, to be independent of the tool Thomas> used to get the file size. For example:
Thomas> fprintf (stderr, "%s: Can't get size of %s: %s\n",

Ahh yes, good idea.

From d7c4cb9f290e22d3fc97e43816158c9fd744200c Mon Sep 17 00:00:00 2001
From: Peter Korsgaard <[email protected]>
Date: Mon, 1 Dec 2008 17:13:17 +0100
Subject: [PATCH] tools/mkimage: use lseek rather than fstat for file size for -l option

Quoted 5 lines

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.

Hm... but lseek() on /dev/mtdblockN will return the size of the MTD device, not of the image that may be stored in it, right?

Later, you should get data checksum errors because of the incorrect lenght, i. e. a ``ERROR: "<image>" has corrupted data!'' error message.

I don't think this works.

tools/mkimage.c NXTARG: ;

Quoted 5 lines
tools/mkimage.c +3 -2
-		if (fstat(ifd, &sbuf) < 0) {
-			fprintf (stderr, "%s: Can't stat %s: %s\n",
+		sbuf.st_size = lseek(ifd, 0, SEEK_END);
+		if (sbuf.st_size == (off_t)-1) {
+			fprintf (stderr, "%s: Can't get size of %s: %s\n",

If you don't use *stat(), you should not use any struct stat type either.

Best regards,

Wolfgang Denk

Signature · 5 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
Der Horizont vieler Menschen ist ein Kreis mit Radius Null --
und das nennen sie ihren Standpunkt.
#5 lore raw

PATCH 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> Hm... but lseek() on /dev/mtdblockN will return the size of Wolfgang> the MTD device, not of the image that may be stored in it, Wolfgang> right?

Yes.

Wolfgang> Later, you should get data checksum errors because of the Wolfgang> incorrect lenght, i. e. a ``ERROR: "<image>" has Wolfgang> corrupted data!'' error message.

Exactly, hence my other patch (tools/mkimage: ignore trailing garbage) to only error out if the size is smaller than the size defined in the header.

Wolfgang> I don't think this works.

Quoted 5 lines

tools/mkimage.c NXTARG: ;

Quoted 5 lines
tools/mkimage.c +3 -2
-		if (fstat(ifd, &sbuf) < 0) {
-			fprintf (stderr, "%s: Can't stat %s: %s\n",
+		sbuf.st_size = lseek(ifd, 0, SEEK_END);
+		if (sbuf.st_size == (off_t)-1) {
+			fprintf (stderr, "%s: Can't get size of %s: %s\n",

Wolfgang> If you don't use *stat(), you should not use any struct Wolfgang> stat type either.

Ok, I kept it like that to keep the patch as minimal as possible - I'll change it and resend.

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

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

Dear Peter Korsgaard,

Quoted 8 lines · In message <[email protected]> you wrote:

Wolfgang> Later, you should get data checksum errors because of the Wolfgang> incorrect lenght, i. e. a ``ERROR: "<image>" has Wolfgang> corrupted data!'' error message.

Exactly, hence my other patch (tools/mkimage: ignore trailing garbage) to only error out if the size is smaller than the size defined in the header.

This doesn't make sense to me. If we have a way to check the image, we want to check everything, including correct image length.

Quoted 5 lines

Wolfgang> If you don't use *stat(), you should not use any struct Wolfgang> stat type either.

Ok, I kept it like that to keep the patch as minimal as possible - I'll change it and resend.

I think it makes little sense as my feeling is the changes make mkimage checking not better, just worse.

Best regards,

Wolfgang Denk

Signature · 5 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
Honest error is to be pitied, not ridiculed.
                                       -- Philip Earl of Chesterfield
#7 lore raw

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

Quoted 3 lines
Quoted 3 lines

Exactly, hence my other patch (tools/mkimage: ignore trailing garbage) to only error out if the size is smaller than the size defined in the header.

Wolfgang> This doesn't make sense to me. If we have a way to check Wolfgang> the image, we want to check everything, including correct Wolfgang> image length.

I provided a reason for the change in the patch:

"E.G. if the uImage was stored in flash and hence padded to the flash sector size"

I can add a warning if there's trailing garbage if you prefer, but I think the use case is valid.

Quoted 2 lines
Quoted 2 lines

Ok, I kept it like that to keep the patch as minimal as possible - I'll change it and resend.

Wolfgang> I think it makes little sense as my feeling is the changes make Wolfgang> mkimage checking not better, just worse.

Ok, fixed in the v2 patch I recently sent.

Signature · 1 line
Bye, Peter Korsgaard