[PATCH v2 1/4] scripts/kwbimage: fix handling of binary header

[PATCH v2 1/4] scripts/kwbimage: fix handling of binary header

[Uwe Kleine-König]

A binary header is 12 bytes + (4 bytes * Number of Arguments) bigger
than the actual binary. Before this commit image extraction was wrong an
made binary.0 too big by four bytes at the end (which were 0 in all usual
cases). Image creation had the same problem which resulted in broken
images when the binary doesn't end in 4 bytes containing 0.

Further handle binaries with a length that is not aligned to 4 bytes.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1:
 - make it patch 1 as it is a fix

 scripts/kwbimage.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/kwbimage.c b/scripts/kwbimage.c
index 448ac2a5d416..c560f581a727 100644
--- a/scripts/kwbimage.c
+++ b/scripts/kwbimage.c
@@ -490,7 +490,7 @@ static int image_extract_binary_hdr_v1(const void *binary, const char *output,
 	}
 
 	ret = fwrite(binary + (nargs + 1) * sizeof(unsigned int),
-		     binsz - (nargs + 1) * sizeof(unsigned int), 1,
+		     binsz - (nargs + 2) * sizeof(unsigned int), 1,
 		     binaryout);
 	if (ret != 1) {
 		fprintf(stderr, "Could not write to output file %s\n",
@@ -869,8 +869,8 @@ static void *image_create_v1(struct image_cfg_element *image_cfg,
 			return NULL;
 		}
 
-		headersz += s.st_size +
-			binarye->binary.nargs * sizeof(unsigned int);
+		headersz += ALIGN_SUP(s.st_size, 4) +
+			12 + binarye->binary.nargs * sizeof(unsigned int);
 		hasext = 1;
 	}
 
@@ -951,8 +951,8 @@ static void *image_create_v1(struct image_cfg_element *image_cfg,
 		fstat(fileno(bin), &s);
 
 		binhdrsz = sizeof(struct opt_hdr_v1) +
-			(binarye->binary.nargs + 1) * sizeof(unsigned int) +
-			s.st_size;
+			(binarye->binary.nargs + 2) * sizeof(unsigned int) +
+			ALIGN_SUP(s.st_size, 4);
 		hdr->headersz_lsb = binhdrsz & 0xFFFF;
 		hdr->headersz_msb = (binhdrsz & 0xFFFF0000) >> 16;
 
@@ -976,7 +976,7 @@ static void *image_create_v1(struct image_cfg_element *image_cfg,
 
 		fclose(bin);
 
-		cur += s.st_size;
+		cur += ALIGN_SUP(s.st_size, 4);
 
 		/*
 		 * For now, we don't support more than one binary
-- 
2.10.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 2/4] scripts/kwbimage: fix typo

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1:
 - add S-o-b

 scripts/kwbimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kwbimage.c b/scripts/kwbimage.c
index c560f581a727..79ca85b72d8e 100644
--- a/scripts/kwbimage.c
+++ b/scripts/kwbimage.c
@@ -523,7 +523,7 @@ static int image_extract_v1(void *fdimap, const char *output, FILE *focfg)
 	int opthdrid;
 
 	/*
-	 * Verify the checkum. We have to substract the checksum
+	 * Verify the checksum. We have to subtract the checksum
 	 * itself, because when the checksum is calculated, the
 	 * checksum field is 0.
 	 */
-- 
2.10.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 3/4] scripts/kwbimage: use ALIGN_SUP instead of open-coding it

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1:
 - add S-o-b

 scripts/kwbimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kwbimage.c b/scripts/kwbimage.c
index 79ca85b72d8e..2af96f055c7e 100644
--- a/scripts/kwbimage.c
+++ b/scripts/kwbimage.c
@@ -886,7 +886,7 @@ static void *image_create_v1(struct image_cfg_element *image_cfg,
 		}
 
 		/* payload size must be multiple of 32b */
-		payloadsz = 4 * ((s.st_size + 3)/4);
+		payloadsz = ALIGN_SUP(s.st_size, 4);
 	}
 
 	/* The payload should be aligned on some reasonable
-- 
2.10.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 4/4] scripts/kwbimage: allow to overwrite binary

[Uwe Kleine-König]

This is a preparation to let barebox provide the binary.0.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
This is new in this series and a left over from my efforts to let
barebox implement the functionality of binary.0. For now that failed,
but this patch should be fine anyhow.

Best regards
Uwe

 scripts/kwbimage.c | 40 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/scripts/kwbimage.c b/scripts/kwbimage.c
index 2af96f055c7e..5b84db3f7a23 100644
--- a/scripts/kwbimage.c
+++ b/scripts/kwbimage.c
@@ -347,6 +347,7 @@ static void usage(const char *prog)
 	printf("   -h: this help text\n");
 	printf(" Options specific to image creation:\n");
 	printf("   -p: path to payload image. Overrides the PAYLOAD line from kwbimage.cfg\n");
+	printf("   -b: path to binary image. Overrides the BINARY line from kwbimage.cfg\n");
 	printf("   -m: boot media. Overrides the BOOT_FROM line from kwbimage.cfg\n");
 	printf("   -d: load address. Overrides the DEST_ADDR line from kwbimage.cfg\n");
 	printf("   -e: exec address. Overrides the EXEC_ADDR line from kwbimage.cfg\n");
@@ -1186,6 +1187,30 @@ static int image_override_payload(struct image_cfg_element *image_cfg,
 	return 0;
 }
 
+static int image_override_binary(struct image_cfg_element *image_cfg,
+				 int *cfgn, char *binary)
+{
+	struct image_cfg_element *e;
+	int cfgi = *cfgn;
+
+	if (!binary)
+		return 0;
+
+	e = image_find_option(image_cfg, *cfgn, IMAGE_CFG_BINARY);
+	if (e) {
+		e->binary.file = binary;
+		return 0;
+	}
+
+	image_cfg[cfgi].type = IMAGE_CFG_BINARY;
+	image_cfg[cfgi].binary.file = binary;
+	image_cfg[cfgi].binary.nargs = 0;
+	cfgi++;
+
+	*cfgn = cfgi;
+	return 0;
+}
+
 static int image_override_bootmedia(struct image_cfg_element *image_cfg,
 				    int *cfgn, const char *bootmedia)
 {
@@ -1332,9 +1357,9 @@ static void image_dump_config(struct image_cfg_element *image_cfg,
 }
 
 static int image_create(const char *input, const char *output,
-			const char *payload, const char *bootmedia,
-			uint32_t dstaddr, uint32_t execaddr,
-			int verbose)
+			const char *payload, char *binary,
+			const char *bootmedia, uint32_t dstaddr,
+			uint32_t execaddr, int verbose)
 {
 	struct image_cfg_element *image_cfg;
 	FILE *outputimg;
@@ -1361,6 +1386,7 @@ static int image_create(const char *input, const char *output,
 	}
 
 	image_override_payload(image_cfg, &cfgn, payload);
+	image_override_binary(image_cfg, &cfgn, binary);
 	image_override_bootmedia(image_cfg, &cfgn, bootmedia);
 	image_override_dstaddr(image_cfg, &cfgn, dstaddr);
 	image_override_execaddr(image_cfg, &cfgn, execaddr);
@@ -1433,9 +1459,10 @@ int main(int argc, char *argv[])
 	int action = -1, opt, verbose = 0;
 	const char *input = NULL, *output = NULL,
 		*payload = NULL, *bootmedia = NULL;
+	char *binary = NULL;
 	uint32_t execaddr = ADDR_INVALID, dstaddr = ADDR_INVALID;
 
-	while ((opt = getopt(argc, argv, "hxci:o:p:m:e:d:v")) != -1) {
+	while ((opt = getopt(argc, argv, "hxci:o:p:b:m:e:d:v")) != -1) {
 		switch (opt) {
 		case 'x':
 			action = ACTION_EXTRACT;
@@ -1452,6 +1479,9 @@ int main(int argc, char *argv[])
 		case 'p':
 			payload = optarg;
 			break;
+		case 'b':
+			binary = optarg;
+			break;
 		case 'm':
 			bootmedia = optarg;
 			break;
@@ -1502,7 +1532,7 @@ int main(int argc, char *argv[])
 	case ACTION_EXTRACT:
 		return image_extract(input, output);
 	case ACTION_CREATE:
-		return image_create(input, output, payload,
+		return image_create(input, output, payload, binary,
 				    bootmedia, dstaddr, execaddr,
 				    verbose);
 	case ACTION_HELP:
-- 
2.10.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/4] scripts/kwbimage: fix handling of binary header

[Sascha Hauer]

On Wed, Nov 16, 2016 at 10:52:41AM +0100, Uwe Kleine-König wrote:
> A binary header is 12 bytes + (4 bytes * Number of Arguments) bigger
> than the actual binary. Before this commit image extraction was wrong an
> made binary.0 too big by four bytes at the end (which were 0 in all usual
> cases). Image creation had the same problem which resulted in broken
> images when the binary doesn't end in 4 bytes containing 0.
> 
> Further handle binaries with a length that is not aligned to 4 bytes.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied, thanks

Sascha

> ---
> Changes since (implicit) v1:
>  - make it patch 1 as it is a fix
> 
>  scripts/kwbimage.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/kwbimage.c b/scripts/kwbimage.c
> index 448ac2a5d416..c560f581a727 100644
> --- a/scripts/kwbimage.c
> +++ b/scripts/kwbimage.c
> @@ -490,7 +490,7 @@ static int image_extract_binary_hdr_v1(const void *binary, const char *output,
>  	}
>  
>  	ret = fwrite(binary + (nargs + 1) * sizeof(unsigned int),
> -		     binsz - (nargs + 1) * sizeof(unsigned int), 1,
> +		     binsz - (nargs + 2) * sizeof(unsigned int), 1,
>  		     binaryout);
>  	if (ret != 1) {
>  		fprintf(stderr, "Could not write to output file %s\n",
> @@ -869,8 +869,8 @@ static void *image_create_v1(struct image_cfg_element *image_cfg,
>  			return NULL;
>  		}
>  
> -		headersz += s.st_size +
> -			binarye->binary.nargs * sizeof(unsigned int);
> +		headersz += ALIGN_SUP(s.st_size, 4) +
> +			12 + binarye->binary.nargs * sizeof(unsigned int);
>  		hasext = 1;
>  	}
>  
> @@ -951,8 +951,8 @@ static void *image_create_v1(struct image_cfg_element *image_cfg,
>  		fstat(fileno(bin), &s);
>  
>  		binhdrsz = sizeof(struct opt_hdr_v1) +
> -			(binarye->binary.nargs + 1) * sizeof(unsigned int) +
> -			s.st_size;
> +			(binarye->binary.nargs + 2) * sizeof(unsigned int) +
> +			ALIGN_SUP(s.st_size, 4);
>  		hdr->headersz_lsb = binhdrsz & 0xFFFF;
>  		hdr->headersz_msb = (binhdrsz & 0xFFFF0000) >> 16;
>  
> @@ -976,7 +976,7 @@ static void *image_create_v1(struct image_cfg_element *image_cfg,
>  
>  		fclose(bin);
>  
> -		cur += s.st_size;
> +		cur += ALIGN_SUP(s.st_size, 4);
>  
>  		/*
>  		 * For now, we don't support more than one binary
> -- 
> 2.10.2
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox