[PATCH] mkimage: Call verify_header after writing image to disk

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH] mkimage: Call verify_header after writing image to disk
@ 2022-01-13 14:42 Pali Rohár
  2022-01-13 19:30 ` Simon Glass
  0 siblings, 1 reply; 21+ messages in thread
From: Pali Rohár @ 2022-01-13 14:42 UTC (permalink / raw)
  To: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún
  Cc: u-boot

If image backend provides verify_header callback then call it after writing
image to disk. This ensures that written image is correct.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/mkimage.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index fbe883ce3620..2d282630787c 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -698,6 +698,40 @@ int main(int argc, char **argv)
 		exit (EXIT_FAILURE);
 	}
 
+	if (tparams->verify_header) {
+		ifd = open(params.imagefile, O_RDONLY | O_BINARY);
+		if (ifd < 0) {
+			fprintf(stderr, "%s: Can't open %s: %s\n",
+				params.cmdname, params.imagefile,
+				strerror(errno));
+			exit(EXIT_FAILURE);
+		}
+
+		if (fstat(ifd, &sbuf) < 0) {
+			fprintf(stderr, "%s: Can't stat %s: %s\n",
+				params.cmdname, params.imagefile, strerror(errno));
+			exit(EXIT_FAILURE);
+		}
+		params.file_size = sbuf.st_size;
+
+		map_len = sbuf.st_size;
+		ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, ifd, 0);
+		if (ptr == MAP_FAILED) {
+			fprintf(stderr, "%s: Can't map %s: %s\n",
+				params.cmdname, params.imagefile, strerror(errno));
+			exit(EXIT_FAILURE);
+		}
+
+		if (tparams->verify_header((unsigned char *)ptr, sbuf.st_size, &params) != 0) {
+			fprintf(stderr, "%s: Failed to verify header of %s\n",
+				params.cmdname, params.imagefile);
+			exit(EXIT_FAILURE);
+		}
+
+		(void)munmap((void *)ptr, map_len);
+		close(ifd);
+	}
+
 	exit (EXIT_SUCCESS);
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] mkimage: Call verify_header after writing image to disk
  2022-01-13 14:42 [PATCH] mkimage: Call verify_header after writing image to disk Pali Rohár
@ 2022-01-13 19:30 ` Simon Glass
  2022-01-14 17:34   ` [PATCH v2] tools: " Pali Rohár
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2022-01-13 19:30 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Alexandru Gagniuc, Yann Dirson, Stefan Roese, Marek Behún,
	U-Boot Mailing List

Hi Pali,

On Thu, 13 Jan 2022 at 07:42, Pali Rohár <pali@kernel.org> wrote:
>
> If image backend provides verify_header callback then call it after writing
> image to disk. This ensures that written image is correct.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  tools/mkimage.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index fbe883ce3620..2d282630787c 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -698,6 +698,40 @@ int main(int argc, char **argv)
>                 exit (EXIT_FAILURE);
>         }
>
> +       if (tparams->verify_header) {
> +               ifd = open(params.imagefile, O_RDONLY | O_BINARY);
> +               if (ifd < 0) {
> +                       fprintf(stderr, "%s: Can't open %s: %s\n",
> +                               params.cmdname, params.imagefile,
> +                               strerror(errno));
> +                       exit(EXIT_FAILURE);
> +               }
> +
> +               if (fstat(ifd, &sbuf) < 0) {
> +                       fprintf(stderr, "%s: Can't stat %s: %s\n",
> +                               params.cmdname, params.imagefile, strerror(errno));
> +                       exit(EXIT_FAILURE);
> +               }
> +               params.file_size = sbuf.st_size;
> +
> +               map_len = sbuf.st_size;
> +               ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, ifd, 0);
> +               if (ptr == MAP_FAILED) {
> +                       fprintf(stderr, "%s: Can't map %s: %s\n",
> +                               params.cmdname, params.imagefile, strerror(errno));
> +                       exit(EXIT_FAILURE);
> +               }
> +
> +               if (tparams->verify_header((unsigned char *)ptr, sbuf.st_size, &params) != 0) {
> +                       fprintf(stderr, "%s: Failed to verify header of %s\n",
> +                               params.cmdname, params.imagefile);
> +                       exit(EXIT_FAILURE);
> +               }
> +
> +               (void)munmap((void *)ptr, map_len);
> +               close(ifd);
> +       }
> +
>         exit (EXIT_SUCCESS);
>  }

main() is so long already. Could this move into a function?

Regards,
Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-13 19:30 ` Simon Glass
@ 2022-01-14 17:34   ` Pali Rohár
  2022-01-15  9:46     ` Stefan Roese
                       ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Pali Rohár @ 2022-01-14 17:34 UTC (permalink / raw)
  To: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún
  Cc: u-boot

If image backend provides verify_header callback then call it after writing
image to disk. This ensures that written image is correct.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index fbe883ce3620..d5ad0925225c 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -336,6 +336,44 @@ static void process_args(int argc, char **argv)
 		usage("Missing output filename");
 }
 
+static void verify_image(const struct image_type_params *tparams)
+{
+	struct stat sbuf;
+	void *ptr;
+	int ifd;
+
+	ifd = open(params.imagefile, O_RDONLY | O_BINARY);
+	if (ifd < 0) {
+		fprintf(stderr, "%s: Can't open %s: %s\n",
+			params.cmdname, params.imagefile,
+			strerror(errno));
+		exit(EXIT_FAILURE);
+	}
+
+	if (fstat(ifd, &sbuf) < 0) {
+		fprintf(stderr, "%s: Can't stat %s: %s\n",
+			params.cmdname, params.imagefile, strerror(errno));
+		exit(EXIT_FAILURE);
+	}
+	params.file_size = sbuf.st_size;
+
+	ptr = mmap(0, params.file_size, PROT_READ, MAP_SHARED, ifd, 0);
+	if (ptr == MAP_FAILED) {
+		fprintf(stderr, "%s: Can't map %s: %s\n",
+			params.cmdname, params.imagefile, strerror(errno));
+		exit(EXIT_FAILURE);
+	}
+
+	if (tparams->verify_header((unsigned char *)ptr, params.file_size, &params) != 0) {
+		fprintf(stderr, "%s: Failed to verify header of %s\n",
+			params.cmdname, params.imagefile);
+		exit(EXIT_FAILURE);
+	}
+
+	(void)munmap(ptr, params.file_size);
+	(void)close(ifd);
+}
+
 int main(int argc, char **argv)
 {
 	int ifd = -1;
@@ -698,6 +736,9 @@ int main(int argc, char **argv)
 		exit (EXIT_FAILURE);
 	}
 
+	if (tparams->verify_header)
+		verify_image(tparams);
+
 	exit (EXIT_SUCCESS);
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-14 17:34   ` [PATCH v2] tools: " Pali Rohár
@ 2022-01-15  9:46     ` Stefan Roese
  2022-01-15 18:01     ` Simon Glass
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Stefan Roese @ 2022-01-15  9:46 UTC (permalink / raw)
  To: Pali Rohár, Simon Glass, Alexandru Gagniuc, Yann Dirson,
	Marek Behún
  Cc: u-boot

On 1/14/22 18:34, Pali Rohár wrote:
> If image backend provides verify_header callback then call it after writing
> image to disk. This ensures that written image is correct.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
> 
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index fbe883ce3620..d5ad0925225c 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -336,6 +336,44 @@ static void process_args(int argc, char **argv)
>   		usage("Missing output filename");
>   }
>   
> +static void verify_image(const struct image_type_params *tparams)
> +{
> +	struct stat sbuf;
> +	void *ptr;
> +	int ifd;
> +
> +	ifd = open(params.imagefile, O_RDONLY | O_BINARY);
> +	if (ifd < 0) {
> +		fprintf(stderr, "%s: Can't open %s: %s\n",
> +			params.cmdname, params.imagefile,
> +			strerror(errno));
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	if (fstat(ifd, &sbuf) < 0) {
> +		fprintf(stderr, "%s: Can't stat %s: %s\n",
> +			params.cmdname, params.imagefile, strerror(errno));
> +		exit(EXIT_FAILURE);
> +	}
> +	params.file_size = sbuf.st_size;
> +
> +	ptr = mmap(0, params.file_size, PROT_READ, MAP_SHARED, ifd, 0);
> +	if (ptr == MAP_FAILED) {
> +		fprintf(stderr, "%s: Can't map %s: %s\n",
> +			params.cmdname, params.imagefile, strerror(errno));
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	if (tparams->verify_header((unsigned char *)ptr, params.file_size, &params) != 0) {
> +		fprintf(stderr, "%s: Failed to verify header of %s\n",
> +			params.cmdname, params.imagefile);
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	(void)munmap(ptr, params.file_size);
> +	(void)close(ifd);
> +}
> +
>   int main(int argc, char **argv)
>   {
>   	int ifd = -1;
> @@ -698,6 +736,9 @@ int main(int argc, char **argv)
>   		exit (EXIT_FAILURE);
>   	}
>   
> +	if (tparams->verify_header)
> +		verify_image(tparams);
> +
>   	exit (EXIT_SUCCESS);
>   }
>   
> 

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-14 17:34   ` [PATCH v2] tools: " Pali Rohár
  2022-01-15  9:46     ` Stefan Roese
@ 2022-01-15 18:01     ` Simon Glass
  2022-01-21 21:21     ` Tom Rini
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2022-01-15 18:01 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Alexandru Gagniuc, Yann Dirson, Stefan Roese, Marek Behún,
	U-Boot Mailing List

On Fri, 14 Jan 2022 at 10:35, Pali Rohár <pali@kernel.org> wrote:
>
> If image backend provides verify_header callback then call it after writing
> image to disk. This ensures that written image is correct.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Missing change log, BTW.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-14 17:34   ` [PATCH v2] tools: " Pali Rohár
  2022-01-15  9:46     ` Stefan Roese
  2022-01-15 18:01     ` Simon Glass
@ 2022-01-21 21:21     ` Tom Rini
  2022-01-22  1:44       ` Pali Rohár
  2022-03-06 11:50       ` Pali Rohár
  2022-03-08 13:42     ` Tom Rini
  2022-04-06 15:55     ` Tom Rini
  4 siblings, 2 replies; 21+ messages in thread
From: Tom Rini @ 2022-01-21 21:21 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]

On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:

> If image backend provides verify_header callback then call it after writing
> image to disk. This ensures that written image is correct.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)

This breaks a number of platforms such as ls1021atwr_sdcard_qspi and
it's not clear to me why exactly.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-21 21:21     ` Tom Rini
@ 2022-01-22  1:44       ` Pali Rohár
  2022-01-22  2:15         ` Tom Rini
  2022-03-06 11:50       ` Pali Rohár
  1 sibling, 1 reply; 21+ messages in thread
From: Pali Rohár @ 2022-01-22  1:44 UTC (permalink / raw)
  To: Tom Rini
  Cc: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot

On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> 
> > If image backend provides verify_header callback then call it after writing
> > image to disk. This ensures that written image is correct.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Reviewed-by: Stefan Roese <sr@denx.de>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > ---
> >  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> 
> This breaks a number of platforms such as ls1021atwr_sdcard_qspi and
> it's not clear to me why exactly.

Maybe they were already broken and this patch just detected it?
Or verify_header callback for particular image type is reject valid
image?

Do you have some pointers to failed build logs?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-22  1:44       ` Pali Rohár
@ 2022-01-22  2:15         ` Tom Rini
  2022-01-22 16:31           ` Pali Rohár
  0 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2022-01-22  2:15 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

On Sat, Jan 22, 2022 at 02:44:22AM +0100, Pali Rohár wrote:
> On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> > 
> > > If image backend provides verify_header callback then call it after writing
> > > image to disk. This ensures that written image is correct.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Reviewed-by: Stefan Roese <sr@denx.de>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 41 insertions(+)
> > 
> > This breaks a number of platforms such as ls1021atwr_sdcard_qspi and
> > it's not clear to me why exactly.
> 
> Maybe they were already broken and this patch just detected it?
> Or verify_header callback for particular image type is reject valid
> image?
> 
> Do you have some pointers to failed build logs?

Try building for ls1021atwr_sdcard_qspi with your patch applied, the
only new thing that's shown in the logs is the error message.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-22  2:15         ` Tom Rini
@ 2022-01-22 16:31           ` Pali Rohár
  2022-01-22 16:34             ` Pali Rohár
  2022-01-22 16:35             ` Tom Rini
  0 siblings, 2 replies; 21+ messages in thread
From: Pali Rohár @ 2022-01-22 16:31 UTC (permalink / raw)
  To: Tom Rini
  Cc: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot

On Friday 21 January 2022 21:15:43 Tom Rini wrote:
> On Sat, Jan 22, 2022 at 02:44:22AM +0100, Pali Rohár wrote:
> > On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> > > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> > > 
> > > > If image backend provides verify_header callback then call it after writing
> > > > image to disk. This ensures that written image is correct.
> > > > 
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > Reviewed-by: Stefan Roese <sr@denx.de>
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 41 insertions(+)
> > > 
> > > This breaks a number of platforms such as ls1021atwr_sdcard_qspi and
> > > it's not clear to me why exactly.
> > 
> > Maybe they were already broken and this patch just detected it?
> > Or verify_header callback for particular image type is reject valid
> > image?
> > 
> > Do you have some pointers to failed build logs?
> 
> Try building for ls1021atwr_sdcard_qspi with your patch applied, the
> only new thing that's shown in the logs is the error message.

So... I have tried following without this patch:

$ make ls1021atwr_sdcard_qspi_defconfig
$ make CROSS_COMPILE=arm-linux-gnueabi- -j8

It generated file spl/u-boot-spl.pbl without error. Now I called -l on
this generated file for type pblimage and I got following output:

$ ./tools/dumpimage -T pblimage -l spl/u-boot-spl.pbl
GP Header: Size aa55aa55 LoadAddr 1ee0100

$ ./tools/mkimage -T pblimage -l spl/u-boot-spl.pbl
GP Header: Size aa55aa55 LoadAddr 1ee0100

"GP Header:" line is from the TI OMAP image backend type gpimage or type
omapimage (implemented in file gpimage-common.c).

So it means that files generated by ls1021atwr_sdcard_qspi are already
broken and my patch just detected it. Or it is also possible that
validation code in pblimage.c file is incorrect and broken.

What to do with it now?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-22 16:31           ` Pali Rohár
@ 2022-01-22 16:34             ` Pali Rohár
  2022-01-22 16:35             ` Tom Rini
  1 sibling, 0 replies; 21+ messages in thread
From: Pali Rohár @ 2022-01-22 16:34 UTC (permalink / raw)
  To: Tom Rini
  Cc: Simon Glass, Alexandru Gagniuc, Stefan Roese, Marek Behún, u-boot

On Saturday 22 January 2022 17:31:18 Pali Rohár wrote:
> On Friday 21 January 2022 21:15:43 Tom Rini wrote:
> > On Sat, Jan 22, 2022 at 02:44:22AM +0100, Pali Rohár wrote:
> > > On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> > > > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> > > > 
> > > > > If image backend provides verify_header callback then call it after writing
> > > > > image to disk. This ensures that written image is correct.
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > Reviewed-by: Stefan Roese <sr@denx.de>
> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 41 insertions(+)
> > > > 
> > > > This breaks a number of platforms such as ls1021atwr_sdcard_qspi and
> > > > it's not clear to me why exactly.
> > > 
> > > Maybe they were already broken and this patch just detected it?
> > > Or verify_header callback for particular image type is reject valid
> > > image?
> > > 
> > > Do you have some pointers to failed build logs?
> > 
> > Try building for ls1021atwr_sdcard_qspi with your patch applied, the
> > only new thing that's shown in the logs is the error message.
> 
> So... I have tried following without this patch:
> 
> $ make ls1021atwr_sdcard_qspi_defconfig
> $ make CROSS_COMPILE=arm-linux-gnueabi- -j8
> 
> It generated file spl/u-boot-spl.pbl without error. Now I called -l on
> this generated file for type pblimage and I got following output:
> 
> $ ./tools/dumpimage -T pblimage -l spl/u-boot-spl.pbl
> GP Header: Size aa55aa55 LoadAddr 1ee0100
> 
> $ ./tools/mkimage -T pblimage -l spl/u-boot-spl.pbl
> GP Header: Size aa55aa55 LoadAddr 1ee0100
> 
> "GP Header:" line is from the TI OMAP image backend type gpimage or type
> omapimage (implemented in file gpimage-common.c).
> 
> So it means that files generated by ls1021atwr_sdcard_qspi are already
> broken and my patch just detected it. Or it is also possible that
> validation code in pblimage.c file is incorrect and broken.

Just to note validation is failing on this tools/pblimage.c check:

  if (pbl_hdr->rcwheader != reverse_byte(RCW_HEADER))

> What to do with it now?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-22 16:31           ` Pali Rohár
  2022-01-22 16:34             ` Pali Rohár
@ 2022-01-22 16:35             ` Tom Rini
  2022-02-02  9:06               ` Priyanka Jain
  1 sibling, 1 reply; 21+ messages in thread
From: Tom Rini @ 2022-01-22 16:35 UTC (permalink / raw)
  To: Pali Rohár, Alison Wang, Priyanka Jain, Mingkai Hu, Rajesh Bhagat
  Cc: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot

[-- Attachment #1: Type: text/plain, Size: 2319 bytes --]

On Sat, Jan 22, 2022 at 05:31:18PM +0100, Pali Rohár wrote:
> On Friday 21 January 2022 21:15:43 Tom Rini wrote:
> > On Sat, Jan 22, 2022 at 02:44:22AM +0100, Pali Rohár wrote:
> > > On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> > > > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> > > > 
> > > > > If image backend provides verify_header callback then call it after writing
> > > > > image to disk. This ensures that written image is correct.
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > Reviewed-by: Stefan Roese <sr@denx.de>
> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 41 insertions(+)
> > > > 
> > > > This breaks a number of platforms such as ls1021atwr_sdcard_qspi and
> > > > it's not clear to me why exactly.
> > > 
> > > Maybe they were already broken and this patch just detected it?
> > > Or verify_header callback for particular image type is reject valid
> > > image?
> > > 
> > > Do you have some pointers to failed build logs?
> > 
> > Try building for ls1021atwr_sdcard_qspi with your patch applied, the
> > only new thing that's shown in the logs is the error message.
> 
> So... I have tried following without this patch:
> 
> $ make ls1021atwr_sdcard_qspi_defconfig
> $ make CROSS_COMPILE=arm-linux-gnueabi- -j8
> 
> It generated file spl/u-boot-spl.pbl without error. Now I called -l on
> this generated file for type pblimage and I got following output:
> 
> $ ./tools/dumpimage -T pblimage -l spl/u-boot-spl.pbl
> GP Header: Size aa55aa55 LoadAddr 1ee0100
> 
> $ ./tools/mkimage -T pblimage -l spl/u-boot-spl.pbl
> GP Header: Size aa55aa55 LoadAddr 1ee0100
> 
> "GP Header:" line is from the TI OMAP image backend type gpimage or type
> omapimage (implemented in file gpimage-common.c).
> 
> So it means that files generated by ls1021atwr_sdcard_qspi are already
> broken and my patch just detected it. Or it is also possible that
> validation code in pblimage.c file is incorrect and broken.
> 
> What to do with it now?

Thanks for digging.  This is a problem for a number of the ls1021,
ls1043 and ls1046 platforms, so lets add some maintainers there.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-22 16:35             ` Tom Rini
@ 2022-02-02  9:06               ` Priyanka Jain
  2022-02-15 19:52                 ` Pali Rohár
  0 siblings, 1 reply; 21+ messages in thread
From: Priyanka Jain @ 2022-02-02  9:06 UTC (permalink / raw)
  To: Tom Rini, Pali Rohár, Alison Wang, Mingkai Hu,
	Rajesh Bhagat, Jiafei Pan
  Cc: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot



>-----Original Message-----
>From: Tom Rini <trini@konsulko.com>
>Sent: Saturday, January 22, 2022 10:05 PM
>To: Pali Rohár <pali@kernel.org>; Alison Wang <alison.wang@nxp.com>;
>Priyanka Jain <priyanka.jain@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
>Rajesh Bhagat <rajesh.bhagat@nxp.com>
>Cc: Simon Glass <sjg@chromium.org>; Alexandru Gagniuc
><mr.nuke.me@gmail.com>; Yann Dirson <yann@blade-group.com>; Stefan
>Roese <sr@denx.de>; Marek Behún <marek.behun@nic.cz>; u-
>boot@lists.denx.de
>Subject: Re: [PATCH v2] tools: mkimage: Call verify_header after writing image
>to disk
>
>On Sat, Jan 22, 2022 at 05:31:18PM +0100, Pali Rohár wrote:
>> On Friday 21 January 2022 21:15:43 Tom Rini wrote:
>> > On Sat, Jan 22, 2022 at 02:44:22AM +0100, Pali Rohár wrote:
>> > > On Friday 21 January 2022 16:21:33 Tom Rini wrote:
>> > > > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
>> > > >
>> > > > > If image backend provides verify_header callback then call it
>> > > > > after writing image to disk. This ensures that written image is correct.
>> > > > >
>> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
>> > > > > Reviewed-by: Stefan Roese <sr@denx.de>
>> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
>> > > > > ---
>> > > > >  tools/mkimage.c | 41
>> > > > > +++++++++++++++++++++++++++++++++++++++++
>> > > > >  1 file changed, 41 insertions(+)
>> > > >
>> > > > This breaks a number of platforms such as ls1021atwr_sdcard_qspi
>> > > > and it's not clear to me why exactly.
>> > >
>> > > Maybe they were already broken and this patch just detected it?
>> > > Or verify_header callback for particular image type is reject
>> > > valid image?
>> > >
>> > > Do you have some pointers to failed build logs?
>> >
>> > Try building for ls1021atwr_sdcard_qspi with your patch applied, the
>> > only new thing that's shown in the logs is the error message.
>>
>> So... I have tried following without this patch:
>>
>> $ make ls1021atwr_sdcard_qspi_defconfig $ make
>> CROSS_COMPILE=arm-linux-gnueabi- -j8
>>
>> It generated file spl/u-boot-spl.pbl without error. Now I called -l on
>> this generated file for type pblimage and I got following output:
>>
>> $ ./tools/dumpimage -T pblimage -l spl/u-boot-spl.pbl GP Header: Size
>> aa55aa55 LoadAddr 1ee0100
>>
>> $ ./tools/mkimage -T pblimage -l spl/u-boot-spl.pbl GP Header: Size
>> aa55aa55 LoadAddr 1ee0100
>>
>> "GP Header:" line is from the TI OMAP image backend type gpimage or
>> type omapimage (implemented in file gpimage-common.c).
>>
>> So it means that files generated by ls1021atwr_sdcard_qspi are already
>> broken and my patch just detected it. Or it is also possible that
>> validation code in pblimage.c file is incorrect and broken.
>>
>> What to do with it now?
>
>Thanks for digging.  This is a problem for a number of the ls1021,
>ls1043 and ls1046 platforms, so lets add some maintainers there.
>
>--
>Tom


I will ask NXP-platform owners to check on this.

Thanks
Priyanka

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-02-02  9:06               ` Priyanka Jain
@ 2022-02-15 19:52                 ` Pali Rohár
  2022-02-16  9:33                   ` Priyanka Jain
  2022-02-16 14:17                   ` Z.Q. Hou
  0 siblings, 2 replies; 21+ messages in thread
From: Pali Rohár @ 2022-02-15 19:52 UTC (permalink / raw)
  To: Priyanka Jain
  Cc: Tom Rini, Alison Wang, Mingkai Hu, Rajesh Bhagat, Jiafei Pan,
	Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot

On Wednesday 02 February 2022 09:06:30 Priyanka Jain wrote:
> >-----Original Message-----
> >From: Tom Rini <trini@konsulko.com>
> >Sent: Saturday, January 22, 2022 10:05 PM
> >To: Pali Rohár <pali@kernel.org>; Alison Wang <alison.wang@nxp.com>;
> >Priyanka Jain <priyanka.jain@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> >Rajesh Bhagat <rajesh.bhagat@nxp.com>
> >Cc: Simon Glass <sjg@chromium.org>; Alexandru Gagniuc
> ><mr.nuke.me@gmail.com>; Yann Dirson <yann@blade-group.com>; Stefan
> >Roese <sr@denx.de>; Marek Behún <marek.behun@nic.cz>; u-
> >boot@lists.denx.de
> >Subject: Re: [PATCH v2] tools: mkimage: Call verify_header after writing image
> >to disk
> >
> >On Sat, Jan 22, 2022 at 05:31:18PM +0100, Pali Rohár wrote:
> >> On Friday 21 January 2022 21:15:43 Tom Rini wrote:
> >> > On Sat, Jan 22, 2022 at 02:44:22AM +0100, Pali Rohár wrote:
> >> > > On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> >> > > > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> >> > > >
> >> > > > > If image backend provides verify_header callback then call it
> >> > > > > after writing image to disk. This ensures that written image is correct.
> >> > > > >
> >> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> >> > > > > Reviewed-by: Stefan Roese <sr@denx.de>
> >> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> >> > > > > ---
> >> > > > >  tools/mkimage.c | 41
> >> > > > > +++++++++++++++++++++++++++++++++++++++++
> >> > > > >  1 file changed, 41 insertions(+)
> >> > > >
> >> > > > This breaks a number of platforms such as ls1021atwr_sdcard_qspi
> >> > > > and it's not clear to me why exactly.
> >> > >
> >> > > Maybe they were already broken and this patch just detected it?
> >> > > Or verify_header callback for particular image type is reject
> >> > > valid image?
> >> > >
> >> > > Do you have some pointers to failed build logs?
> >> >
> >> > Try building for ls1021atwr_sdcard_qspi with your patch applied, the
> >> > only new thing that's shown in the logs is the error message.
> >>
> >> So... I have tried following without this patch:
> >>
> >> $ make ls1021atwr_sdcard_qspi_defconfig $ make
> >> CROSS_COMPILE=arm-linux-gnueabi- -j8
> >>
> >> It generated file spl/u-boot-spl.pbl without error. Now I called -l on
> >> this generated file for type pblimage and I got following output:
> >>
> >> $ ./tools/dumpimage -T pblimage -l spl/u-boot-spl.pbl GP Header: Size
> >> aa55aa55 LoadAddr 1ee0100
> >>
> >> $ ./tools/mkimage -T pblimage -l spl/u-boot-spl.pbl GP Header: Size
> >> aa55aa55 LoadAddr 1ee0100
> >>
> >> "GP Header:" line is from the TI OMAP image backend type gpimage or
> >> type omapimage (implemented in file gpimage-common.c).
> >>
> >> So it means that files generated by ls1021atwr_sdcard_qspi are already
> >> broken and my patch just detected it. Or it is also possible that
> >> validation code in pblimage.c file is incorrect and broken.
> >>
> >> What to do with it now?
> >
> >Thanks for digging.  This is a problem for a number of the ls1021,
> >ls1043 and ls1046 platforms, so lets add some maintainers there.
> >
> >--
> >Tom
> 
> 
> I will ask NXP-platform owners to check on this.
> 
> Thanks
> Priyanka

Hello! Any news on this?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-02-15 19:52                 ` Pali Rohár
@ 2022-02-16  9:33                   ` Priyanka Jain
  2022-02-16 14:17                   ` Z.Q. Hou
  1 sibling, 0 replies; 21+ messages in thread
From: Priyanka Jain @ 2022-02-16  9:33 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Tom Rini, Alison Wang, Mingkai Hu, Rajesh Bhagat, Jiafei Pan,
	Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot, Jiafei Pan, Xiaobo Xie



>-----Original Message-----
>From: Pali Rohár <pali@kernel.org>
>Sent: Wednesday, February 16, 2022 1:22 AM
>To: Priyanka Jain <priyanka.jain@nxp.com>
>Cc: Tom Rini <trini@konsulko.com>; Alison Wang <alison.wang@nxp.com>;
>Mingkai Hu <mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>;
>Jiafei Pan <jiafei.pan@nxp.com>; Simon Glass <sjg@chromium.org>; Alexandru
>Gagniuc <mr.nuke.me@gmail.com>; Yann Dirson <yann@blade-group.com>;
>Stefan Roese <sr@denx.de>; Marek Behún <marek.behun@nic.cz>; u-
>boot@lists.denx.de
>Subject: Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to
>disk
>
>On Wednesday 02 February 2022 09:06:30 Priyanka Jain wrote:
>> >-----Original Message-----
>> >From: Tom Rini <trini@konsulko.com>
>> >Sent: Saturday, January 22, 2022 10:05 PM
>> >To: Pali Rohár <pali@kernel.org>; Alison Wang <alison.wang@nxp.com>;
>> >Priyanka Jain <priyanka.jain@nxp.com>; Mingkai Hu
>> ><mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
>> >Cc: Simon Glass <sjg@chromium.org>; Alexandru Gagniuc
>> ><mr.nuke.me@gmail.com>; Yann Dirson <yann@blade-group.com>; Stefan
>> >Roese <sr@denx.de>; Marek Behún <marek.behun@nic.cz>; u-
>> >boot@lists.denx.de
>> >Subject: Re: [PATCH v2] tools: mkimage: Call verify_header after
>> >writing image to disk
>> >
>> >On Sat, Jan 22, 2022 at 05:31:18PM +0100, Pali Rohár wrote:
>> >> On Friday 21 January 2022 21:15:43 Tom Rini wrote:
>> >> > On Sat, Jan 22, 2022 at 02:44:22AM +0100, Pali Rohár wrote:
>> >> > > On Friday 21 January 2022 16:21:33 Tom Rini wrote:
>> >> > > > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
>> >> > > >
>> >> > > > > If image backend provides verify_header callback then call
>> >> > > > > it after writing image to disk. This ensures that written image is
>correct.
>> >> > > > >
>> >> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
>> >> > > > > Reviewed-by: Stefan Roese <sr@denx.de>
>> >> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
>> >> > > > > ---
>> >> > > > >  tools/mkimage.c | 41
>> >> > > > > +++++++++++++++++++++++++++++++++++++++++
>> >> > > > >  1 file changed, 41 insertions(+)
>> >> > > >
>> >> > > > This breaks a number of platforms such as
>> >> > > > ls1021atwr_sdcard_qspi and it's not clear to me why exactly.
>> >> > >
>> >> > > Maybe they were already broken and this patch just detected it?
>> >> > > Or verify_header callback for particular image type is reject
>> >> > > valid image?
>> >> > >
>> >> > > Do you have some pointers to failed build logs?
>> >> >
>> >> > Try building for ls1021atwr_sdcard_qspi with your patch applied,
>> >> > the only new thing that's shown in the logs is the error message.
>> >>
>> >> So... I have tried following without this patch:
>> >>
>> >> $ make ls1021atwr_sdcard_qspi_defconfig $ make
>> >> CROSS_COMPILE=arm-linux-gnueabi- -j8
>> >>
>> >> It generated file spl/u-boot-spl.pbl without error. Now I called -l
>> >> on this generated file for type pblimage and I got following output:
>> >>
>> >> $ ./tools/dumpimage -T pblimage -l spl/u-boot-spl.pbl GP Header:
>> >> Size
>> >> aa55aa55 LoadAddr 1ee0100
>> >>
>> >> $ ./tools/mkimage -T pblimage -l spl/u-boot-spl.pbl GP Header: Size
>> >> aa55aa55 LoadAddr 1ee0100
>> >>
>> >> "GP Header:" line is from the TI OMAP image backend type gpimage or
>> >> type omapimage (implemented in file gpimage-common.c).
>> >>
>> >> So it means that files generated by ls1021atwr_sdcard_qspi are
>> >> already broken and my patch just detected it. Or it is also
>> >> possible that validation code in pblimage.c file is incorrect and broken.
>> >>
>> >> What to do with it now?
>> >
>> >Thanks for digging.  This is a problem for a number of the ls1021,
>> >ls1043 and ls1046 platforms, so lets add some maintainers there.
>> >
>> >--
>> >Tom
>>
>>
>> I will ask NXP-platform owners to check on this.
>>
>> Thanks
>> Priyanka
>
>Hello! Any news on this?

Not yet, Actually team was busy in some other critical work.
I will try my best to get response as soon as possible.

Thanks
Priyanka



^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-02-15 19:52                 ` Pali Rohár
  2022-02-16  9:33                   ` Priyanka Jain
@ 2022-02-16 14:17                   ` Z.Q. Hou
  1 sibling, 0 replies; 21+ messages in thread
From: Z.Q. Hou @ 2022-02-16 14:17 UTC (permalink / raw)
  To: Pali Rohár, Priyanka Jain
  Cc: Tom Rini, Alison Wang, Mingkai Hu, Rajesh Bhagat, Jiafei Pan,
	Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot, Z.Q. Hou



> -----Original Message-----
> From: Pali Rohár <pali@kernel.org>
> Sent: 2022年2月16日 3:52
> To: Priyanka Jain <priyanka.jain@nxp.com>
> Cc: Tom Rini <trini@konsulko.com>; Alison Wang <alison.wang@nxp.com>;
> Mingkai Hu <mingkai.hu@nxp.com>; Rajesh Bhagat
> <rajesh.bhagat@nxp.com>; Jiafei Pan <jiafei.pan@nxp.com>; Simon Glass
> <sjg@chromium.org>; Alexandru Gagniuc <mr.nuke.me@gmail.com>; Yann
> Dirson <yann@blade-group.com>; Stefan Roese <sr@denx.de>; Marek
> Behún <marek.behun@nic.cz>; u-boot@lists.denx.de
> Subject: Re: [PATCH v2] tools: mkimage: Call verify_header after writing
> image to disk
> 
> On Wednesday 02 February 2022 09:06:30 Priyanka Jain wrote:
> > >-----Original Message-----
> > >From: Tom Rini <trini@konsulko.com>
> > >Sent: Saturday, January 22, 2022 10:05 PM
> > >To: Pali Rohár <pali@kernel.org>; Alison Wang <alison.wang@nxp.com>;
> > >Priyanka Jain <priyanka.jain@nxp.com>; Mingkai Hu
> > ><mingkai.hu@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > >Cc: Simon Glass <sjg@chromium.org>; Alexandru Gagniuc
> > ><mr.nuke.me@gmail.com>; Yann Dirson <yann@blade-group.com>;
> Stefan
> > >Roese <sr@denx.de>; Marek Behún <marek.behun@nic.cz>; u-
> > >boot@lists.denx.de
> > >Subject: Re: [PATCH v2] tools: mkimage: Call verify_header after
> > >writing image to disk
> > >
> > >On Sat, Jan 22, 2022 at 05:31:18PM +0100, Pali Rohár wrote:
> > >> On Friday 21 January 2022 21:15:43 Tom Rini wrote:
> > >> > On Sat, Jan 22, 2022 at 02:44:22AM +0100, Pali Rohár wrote:
> > >> > > On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> > >> > > > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> > >> > > >
> > >> > > > > If image backend provides verify_header callback then call
> > >> > > > > it after writing image to disk. This ensures that written image is
> correct.
> > >> > > > >
> > >> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > >> > > > > Reviewed-by: Stefan Roese <sr@denx.de>
> > >> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > >> > > > > ---
> > >> > > > >  tools/mkimage.c | 41
> > >> > > > > +++++++++++++++++++++++++++++++++++++++++
> > >> > > > >  1 file changed, 41 insertions(+)
> > >> > > >
> > >> > > > This breaks a number of platforms such as
> > >> > > > ls1021atwr_sdcard_qspi and it's not clear to me why exactly.
> > >> > >
> > >> > > Maybe they were already broken and this patch just detected it?
> > >> > > Or verify_header callback for particular image type is reject
> > >> > > valid image?
> > >> > >
> > >> > > Do you have some pointers to failed build logs?
> > >> >
> > >> > Try building for ls1021atwr_sdcard_qspi with your patch applied,
> > >> > the only new thing that's shown in the logs is the error message.
> > >>
> > >> So... I have tried following without this patch:
> > >>
> > >> $ make ls1021atwr_sdcard_qspi_defconfig $ make
> > >> CROSS_COMPILE=arm-linux-gnueabi- -j8
> > >>
> > >> It generated file spl/u-boot-spl.pbl without error. Now I called -l
> > >> on this generated file for type pblimage and I got following output:
> > >>
> > >> $ ./tools/dumpimage -T pblimage -l spl/u-boot-spl.pbl GP Header:
> > >> Size
> > >> aa55aa55 LoadAddr 1ee0100
> > >>
> > >> $ ./tools/mkimage -T pblimage -l spl/u-boot-spl.pbl GP Header: Size
> > >> aa55aa55 LoadAddr 1ee0100
> > >>
> > >> "GP Header:" line is from the TI OMAP image backend type gpimage or
> > >> type omapimage (implemented in file gpimage-common.c).
> > >>
> > >> So it means that files generated by ls1021atwr_sdcard_qspi are
> > >> already broken and my patch just detected it. Or it is also
> > >> possible that validation code in pblimage.c file is incorrect and broken.
> > >>
> > >> What to do with it now?
> > >
> > >Thanks for digging.  This is a problem for a number of the ls1021,
> > >ls1043 and ls1046 platforms, so lets add some maintainers there.
> > >
> > >--
> > >Tom
> >
> >
> > I will ask NXP-platform owners to check on this.
> >
> > Thanks
> > Priyanka
> 
> Hello! Any news on this?

Indeed it is a historical problem and exposed by this patch.
The problem is the RCW headers are different on PPC and ARM platforms, while ls1021a/ls1043a/ls1046a leveraged the PPC tool to generate PBL image, but didn’t handle this differentially, this result in the image header verification failed.
I'll submit a patch to fix it.

Thanks,
Zhiqiang

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-21 21:21     ` Tom Rini
  2022-01-22  1:44       ` Pali Rohár
@ 2022-03-06 11:50       ` Pali Rohár
  2022-03-06 13:58         ` Tom Rini
  1 sibling, 1 reply; 21+ messages in thread
From: Pali Rohár @ 2022-03-06 11:50 UTC (permalink / raw)
  To: Tom Rini
  Cc: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot

On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> 
> > If image backend provides verify_header callback then call it after writing
> > image to disk. This ensures that written image is correct.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Reviewed-by: Stefan Roese <sr@denx.de>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > ---
> >  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> 
> This breaks a number of platforms such as ls1021atwr_sdcard_qspi and
> it's not clear to me why exactly.
> 
> -- 
> Tom

Hello Tom! Is something still blocking this change?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-03-06 11:50       ` Pali Rohár
@ 2022-03-06 13:58         ` Tom Rini
  0 siblings, 0 replies; 21+ messages in thread
From: Tom Rini @ 2022-03-06 13:58 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Sun, Mar 06, 2022 at 12:50:24PM +0100, Pali Rohár wrote:
> On Friday 21 January 2022 16:21:33 Tom Rini wrote:
> > On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
> > 
> > > If image backend provides verify_header callback then call it after writing
> > > image to disk. This ensures that written image is correct.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Reviewed-by: Stefan Roese <sr@denx.de>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 41 insertions(+)
> > 
> > This breaks a number of platforms such as ls1021atwr_sdcard_qspi and
> > it's not clear to me why exactly.
> 
> Hello Tom! Is something still blocking this change?

Nope, thanks for the reminder.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-14 17:34   ` [PATCH v2] tools: " Pali Rohár
                       ` (2 preceding siblings ...)
  2022-01-21 21:21     ` Tom Rini
@ 2022-03-08 13:42     ` Tom Rini
  2022-03-14  1:51       ` 陈健洪
  2022-03-14  1:54       ` 陈健洪
  2022-04-06 15:55     ` Tom Rini
  4 siblings, 2 replies; 21+ messages in thread
From: Tom Rini @ 2022-03-08 13:42 UTC (permalink / raw)
  To: Pali Rohár, Joseph Chen
  Cc: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot

[-- Attachment #1: Type: text/plain, Size: 2322 bytes --]

On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:

> If image backend provides verify_header callback then call it after writing
> image to disk. This ensures that written image is correct.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index fbe883ce3620..d5ad0925225c 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -336,6 +336,44 @@ static void process_args(int argc, char **argv)
>  		usage("Missing output filename");
>  }
>  
> +static void verify_image(const struct image_type_params *tparams)
> +{
> +	struct stat sbuf;
> +	void *ptr;
> +	int ifd;
> +
> +	ifd = open(params.imagefile, O_RDONLY | O_BINARY);
> +	if (ifd < 0) {
> +		fprintf(stderr, "%s: Can't open %s: %s\n",
> +			params.cmdname, params.imagefile,
> +			strerror(errno));
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	if (fstat(ifd, &sbuf) < 0) {
> +		fprintf(stderr, "%s: Can't stat %s: %s\n",
> +			params.cmdname, params.imagefile, strerror(errno));
> +		exit(EXIT_FAILURE);
> +	}
> +	params.file_size = sbuf.st_size;
> +
> +	ptr = mmap(0, params.file_size, PROT_READ, MAP_SHARED, ifd, 0);
> +	if (ptr == MAP_FAILED) {
> +		fprintf(stderr, "%s: Can't map %s: %s\n",
> +			params.cmdname, params.imagefile, strerror(errno));
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	if (tparams->verify_header((unsigned char *)ptr, params.file_size, &params) != 0) {
> +		fprintf(stderr, "%s: Failed to verify header of %s\n",
> +			params.cmdname, params.imagefile);
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	(void)munmap(ptr, params.file_size);
> +	(void)close(ifd);
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	int ifd = -1;
> @@ -698,6 +736,9 @@ int main(int argc, char **argv)
>  		exit (EXIT_FAILURE);
>  	}
>  
> +	if (tparams->verify_header)
> +		verify_image(tparams);
> +
>  	exit (EXIT_SUCCESS);
>  }

I've added Joseph Chen to the thread as with this patch applied,
evb-rk3568 fails to build now with:
./tools/mkimage: Failed to verify header of idbloader.img
which is another issue to resolve.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-03-08 13:42     ` Tom Rini
@ 2022-03-14  1:51       ` 陈健洪
  2022-03-14  1:54       ` 陈健洪
  1 sibling, 0 replies; 21+ messages in thread
From: 陈健洪 @ 2022-03-14  1:51 UTC (permalink / raw)
  To: trini, PaliRohár
  Cc: sjg, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	MarekBehún, u-boot, kever.yang

Hi,
    The rk3568 uses V2 IDB structrure and the rkcommon_verify_header() has not cover V2.
    My teammate will fix it these days and kever.yang will submit his patch to mainline.



陈健洪 (Joseph Chen)     
E-mail：chenjh@rock-chips.com
瑞芯微电子股份有限公司         
Rockchip Electronics Co.Ltd
福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)
No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC
TEL：0591-83991906/07-8573 
 
From: Tom Rini
Date: 2022-03-08 21:42
To: PaliRohár; Joseph Chen
CC: Simon Glass; Alexandru Gagniuc; Yann Dirson; Stefan Roese; MarekBehún; u-boot
Subject: Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
 
> If image backend provides verify_header callback then call it after writing
> image to disk. This ensures that written image is correct.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index fbe883ce3620..d5ad0925225c 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -336,6 +336,44 @@ static void process_args(int argc, char **argv)
>  usage("Missing output filename");
>  }
>  
> +static void verify_image(const struct image_type_params *tparams)
> +{
> + struct stat sbuf;
> + void *ptr;
> + int ifd;
> +
> + ifd = open(params.imagefile, O_RDONLY | O_BINARY);
> + if (ifd < 0) {
> + fprintf(stderr, "%s: Can't open %s: %s\n",
> + params.cmdname, params.imagefile,
> + strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> +
> + if (fstat(ifd, &sbuf) < 0) {
> + fprintf(stderr, "%s: Can't stat %s: %s\n",
> + params.cmdname, params.imagefile, strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> + params.file_size = sbuf.st_size;
> +
> + ptr = mmap(0, params.file_size, PROT_READ, MAP_SHARED, ifd, 0);
> + if (ptr == MAP_FAILED) {
> + fprintf(stderr, "%s: Can't map %s: %s\n",
> + params.cmdname, params.imagefile, strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> +
> + if (tparams->verify_header((unsigned char *)ptr, params.file_size, &params) != 0) {
> + fprintf(stderr, "%s: Failed to verify header of %s\n",
> + params.cmdname, params.imagefile);
> + exit(EXIT_FAILURE);
> + }
> +
> + (void)munmap(ptr, params.file_size);
> + (void)close(ifd);
> +}
> +
>  int main(int argc, char **argv)
>  {
>  int ifd = -1;
> @@ -698,6 +736,9 @@ int main(int argc, char **argv)
>  exit (EXIT_FAILURE);
>  }
>  
> + if (tparams->verify_header)
> + verify_image(tparams);
> +
>  exit (EXIT_SUCCESS);
>  }
 
I've added Joseph Chen to the thread as with this patch applied,
evb-rk3568 fails to build now with:
./tools/mkimage: Failed to verify header of idbloader.img
which is another issue to resolve.
 
-- 
Tom

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-03-08 13:42     ` Tom Rini
  2022-03-14  1:51       ` 陈健洪
@ 2022-03-14  1:54       ` 陈健洪
  1 sibling, 0 replies; 21+ messages in thread
From: 陈健洪 @ 2022-03-14  1:54 UTC (permalink / raw)
  To: trini, PaliRohár
  Cc: sjg, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	MarekBehún, u-boot, 杨凯

Hi,
    The rk3568 uses V2 IDB structrure and the rkcommon_verify_header() has not cover V2.
    My teammate will fix it these days and kever.yang will submit his patch to mainline.



陈健洪 (Joseph Chen)     
E-mail：chenjh@rock-chips.com
瑞芯微电子股份有限公司         
Rockchip Electronics Co.Ltd
福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)
No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC
TEL：0591-83991906/07-8573 
 
From: Tom Rini
Date: 2022-03-08 21:42
To: PaliRohár; Joseph Chen
CC: Simon Glass; Alexandru Gagniuc; Yann Dirson; Stefan Roese; MarekBehún; u-boot
Subject: Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:
 
> If image backend provides verify_header callback then call it after writing
> image to disk. This ensures that written image is correct.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>  tools/mkimage.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index fbe883ce3620..d5ad0925225c 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -336,6 +336,44 @@ static void process_args(int argc, char **argv)
>  usage("Missing output filename");
>  }
>  
> +static void verify_image(const struct image_type_params *tparams)
> +{
> + struct stat sbuf;
> + void *ptr;
> + int ifd;
> +
> + ifd = open(params.imagefile, O_RDONLY | O_BINARY);
> + if (ifd < 0) {
> + fprintf(stderr, "%s: Can't open %s: %s\n",
> + params.cmdname, params.imagefile,
> + strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> +
> + if (fstat(ifd, &sbuf) < 0) {
> + fprintf(stderr, "%s: Can't stat %s: %s\n",
> + params.cmdname, params.imagefile, strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> + params.file_size = sbuf.st_size;
> +
> + ptr = mmap(0, params.file_size, PROT_READ, MAP_SHARED, ifd, 0);
> + if (ptr == MAP_FAILED) {
> + fprintf(stderr, "%s: Can't map %s: %s\n",
> + params.cmdname, params.imagefile, strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> +
> + if (tparams->verify_header((unsigned char *)ptr, params.file_size, &params) != 0) {
> + fprintf(stderr, "%s: Failed to verify header of %s\n",
> + params.cmdname, params.imagefile);
> + exit(EXIT_FAILURE);
> + }
> +
> + (void)munmap(ptr, params.file_size);
> + (void)close(ifd);
> +}
> +
>  int main(int argc, char **argv)
>  {
>  int ifd = -1;
> @@ -698,6 +736,9 @@ int main(int argc, char **argv)
>  exit (EXIT_FAILURE);
>  }
>  
> + if (tparams->verify_header)
> + verify_image(tparams);
> +
>  exit (EXIT_SUCCESS);
>  }
 
I've added Joseph Chen to the thread as with this patch applied,
evb-rk3568 fails to build now with:
./tools/mkimage: Failed to verify header of idbloader.img
which is another issue to resolve.
 
-- 
Tom

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] tools: mkimage: Call verify_header after writing image to disk
  2022-01-14 17:34   ` [PATCH v2] tools: " Pali Rohár
                       ` (3 preceding siblings ...)
  2022-03-08 13:42     ` Tom Rini
@ 2022-04-06 15:55     ` Tom Rini
  4 siblings, 0 replies; 21+ messages in thread
From: Tom Rini @ 2022-04-06 15:55 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Simon Glass, Alexandru Gagniuc, Yann Dirson, Stefan Roese,
	Marek Behún, u-boot

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]

On Fri, Jan 14, 2022 at 06:34:43PM +0100, Pali Rohár wrote:

> If image backend provides verify_header callback then call it after writing
> image to disk. This ensures that written image is correct.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-04-06 15:55 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 14:42 [PATCH] mkimage: Call verify_header after writing image to disk Pali Rohár
2022-01-13 19:30 ` Simon Glass
2022-01-14 17:34   ` [PATCH v2] tools: " Pali Rohár
2022-01-15  9:46     ` Stefan Roese
2022-01-15 18:01     ` Simon Glass
2022-01-21 21:21     ` Tom Rini
2022-01-22  1:44       ` Pali Rohár
2022-01-22  2:15         ` Tom Rini
2022-01-22 16:31           ` Pali Rohár
2022-01-22 16:34             ` Pali Rohár
2022-01-22 16:35             ` Tom Rini
2022-02-02  9:06               ` Priyanka Jain
2022-02-15 19:52                 ` Pali Rohár
2022-02-16  9:33                   ` Priyanka Jain
2022-02-16 14:17                   ` Z.Q. Hou
2022-03-06 11:50       ` Pali Rohár
2022-03-06 13:58         ` Tom Rini
2022-03-08 13:42     ` Tom Rini
2022-03-14  1:51       ` 陈健洪
2022-03-14  1:54       ` 陈健洪
2022-04-06 15:55     ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.