[PATCH u-boot 1/3] tools: imagetool: Fix error message when verify_header is undefined

[PATCH u-boot 1/3] tools: imagetool: Fix error message when verify_header is undefined

[Pali Rohár]

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 tools/imagetool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/imagetool.c b/tools/imagetool.c
index f14ca2fb979f..688169b3a813 100644
--- a/tools/imagetool.c
+++ b/tools/imagetool.c
@@ -105,7 +105,7 @@ static int imagetool_verify_print_header_by_type(
 		}
 
 	} else {
-		fprintf(stderr, "%s: print_header undefined for %s\n",
+		fprintf(stderr, "%s: verify_header undefined for %s\n",
 			params->cmdname, tparams->name);
 	}
 
-- 
2.20.1

[PATCH u-boot 2/3] tools: imagetool: Show error message when detecting image type failed

[Pali Rohár]

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

diff --git a/tools/imagetool.c b/tools/imagetool.c
index 688169b3a813..e1021f44f5ad 100644
--- a/tools/imagetool.c
+++ b/tools/imagetool.c
@@ -71,6 +71,11 @@ int imagetool_verify_print_header(
 		}
 	}
 
+	if (retval != 0) {
+		fprintf(stderr, "%s: cannot detect image type\n",
+			params->cmdname);
+	}
+
 	return retval;
 }
 
-- 
2.20.1

[PATCH u-boot 3/3] tools: imagetool: Skip autodetection of gpimage type

[Pali Rohár]

gpimage type requires only that two first 32-bit words of data file are
non-zero. So basically every random data file can be guessed and verified
as gpimage. So completely skip gpimage type from image autodetection code
to prevent lot of false positive results. Data file with gpimage type can
be still verified and parsed by explicitly specifying -T gpimage.

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

diff --git a/tools/imagetool.c b/tools/imagetool.c
index e1021f44f5ad..87eee4ad04ed 100644
--- a/tools/imagetool.c
+++ b/tools/imagetool.c
@@ -49,6 +49,12 @@ int imagetool_verify_print_header(
 		return imagetool_verify_print_header_by_type(ptr, sbuf, tparams, params);
 
 	for (curr = start; curr != end; curr++) {
+		/*
+		 * Basically every data file can be guessed / verified as gpimage,
+		 * so skip autodetection of data file as gpimage as it does not work.
+		 */
+		if ((*curr)->check_image_type && (*curr)->check_image_type(IH_TYPE_GPIMAGE) == 0)
+			continue;
 		if ((*curr)->verify_header) {
 			retval = (*curr)->verify_header((unsigned char *)ptr,
 						     sbuf->st_size, params);
-- 
2.20.1

Re: [PATCH u-boot 3/3] tools: imagetool: Skip autodetection of gpimage type

[Simon Glass]

On Sun, 29 Jan 2023 at 09:46, Pali Rohár <pali@kernel.org> wrote:
>
> gpimage type requires only that two first 32-bit words of data file are
> non-zero. So basically every random data file can be guessed and verified
> as gpimage. So completely skip gpimage type from image autodetection code
> to prevent lot of false positive results. Data file with gpimage type can
> be still verified and parsed by explicitly specifying -T gpimage.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  tools/imagetool.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>

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

Re: [PATCH u-boot 1/3] tools: imagetool: Fix error message when verify_header is undefined

[Simon Glass]

On Sun, 29 Jan 2023 at 09:46, Pali Rohár <pali@kernel.org> wrote:

commit message? Also it is good to keep subjects to under 60 chars


>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  tools/imagetool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/imagetool.c b/tools/imagetool.c
> index f14ca2fb979f..688169b3a813 100644
> --- a/tools/imagetool.c
> +++ b/tools/imagetool.c
> @@ -105,7 +105,7 @@ static int imagetool_verify_print_header_by_type(
>                 }
>
>         } else {
> -               fprintf(stderr, "%s: print_header undefined for %s\n",
> +               fprintf(stderr, "%s: verify_header undefined for %s\n",
>                         params->cmdname, tparams->name);
>         }
>
> --
> 2.20.1
>

Re: [PATCH u-boot 3/3] tools: imagetool: Skip autodetection of gpimage type

[Tom Rini]

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

On Mon, Jan 30, 2023 at 08:50:15AM -0700, Simon Glass wrote:
> On Sun, 29 Jan 2023 at 09:46, Pali Rohár <pali@kernel.org> wrote:
> >
> > gpimage type requires only that two first 32-bit words of data file are
> > non-zero. So basically every random data file can be guessed and verified
> > as gpimage. So completely skip gpimage type from image autodetection code
> > to prevent lot of false positive results. Data file with gpimage type can
> > be still verified and parsed by explicitly specifying -T gpimage.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  tools/imagetool.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

I see we've had problems with gpimage before too. This seems reasonable
but I'm adding Nishanth here too, as the current interested person in
keystone2 platforms, to see if there's any other / better ways to
address this problem.

-- 
Tom

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

Re: [PATCH u-boot 3/3] tools: imagetool: Skip autodetection of gpimage type

[Pali Rohár]

On Monday 30 January 2023 13:02:42 Tom Rini wrote:
> On Mon, Jan 30, 2023 at 08:50:15AM -0700, Simon Glass wrote:
> > On Sun, 29 Jan 2023 at 09:46, Pali Rohár <pali@kernel.org> wrote:
> > >
> > > gpimage type requires only that two first 32-bit words of data file are
> > > non-zero. So basically every random data file can be guessed and verified
> > > as gpimage. So completely skip gpimage type from image autodetection code
> > > to prevent lot of false positive results. Data file with gpimage type can
> > > be still verified and parsed by explicitly specifying -T gpimage.
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > >  tools/imagetool.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > 
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I see we've had problems with gpimage before too. This seems reasonable
> but I'm adding Nishanth here too, as the current interested person in
> keystone2 platforms, to see if there's any other / better ways to
> address this problem.
> 
> -- 
> Tom

I do not think that there is a better solution for gpimage. Basically it
is not possible to write autodetection code for gpimage due to its
generic nature. So the best what we can do is to disable gpimage in
autodetection code.

I would suggest to apply this patch, so people can test their build
setups sooner than later.

Re: [PATCH u-boot 3/3] tools: imagetool: Skip autodetection of gpimage type

[Tom Rini]

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

On Fri, Feb 03, 2023 at 08:24:20PM +0100, Pali Rohár wrote:
> On Monday 30 January 2023 13:02:42 Tom Rini wrote:
> > On Mon, Jan 30, 2023 at 08:50:15AM -0700, Simon Glass wrote:
> > > On Sun, 29 Jan 2023 at 09:46, Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > gpimage type requires only that two first 32-bit words of data file are
> > > > non-zero. So basically every random data file can be guessed and verified
> > > > as gpimage. So completely skip gpimage type from image autodetection code
> > > > to prevent lot of false positive results. Data file with gpimage type can
> > > > be still verified and parsed by explicitly specifying -T gpimage.
> > > >
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > ---
> > > >  tools/imagetool.c | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > 
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > 
> > I see we've had problems with gpimage before too. This seems reasonable
> > but I'm adding Nishanth here too, as the current interested person in
> > keystone2 platforms, to see if there's any other / better ways to
> > address this problem.
> 
> I do not think that there is a better solution for gpimage. Basically it
> is not possible to write autodetection code for gpimage due to its
> generic nature. So the best what we can do is to disable gpimage in
> autodetection code.
> 
> I would suggest to apply this patch, so people can test their build
> setups sooner than later.

Yes, I will pick this up before -rc2. I was wondering / hoping Nishanth
might have access to further internal information that might be helpful
here.

-- 
Tom

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

Re: [PATCH u-boot 1/3] tools: imagetool: Fix error message when verify_header is undefined

[Tom Rini]

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

On Sun, Jan 29, 2023 at 05:45:53PM +0100, Pali Rohár wrote:

> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

Re: [PATCH u-boot 2/3] tools: imagetool: Show error message when detecting image type failed

[Tom Rini]

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

On Sun, Jan 29, 2023 at 05:45:54PM +0100, Pali Rohár wrote:

> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

Re: [PATCH u-boot 3/3] tools: imagetool: Skip autodetection of gpimage type

[Tom Rini]

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

On Sun, Jan 29, 2023 at 05:45:55PM +0100, Pali Rohár wrote:

> gpimage type requires only that two first 32-bit words of data file are
> non-zero. So basically every random data file can be guessed and verified
> as gpimage. So completely skip gpimage type from image autodetection code
> to prevent lot of false positive results. Data file with gpimage type can
> be still verified and parsed by explicitly specifying -T gpimage.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> 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 --]