[PATCH] image-fit: aligned output of hash calculation

[PATCH] image-fit: aligned output of hash calculation

[Christoph Fritz]

When calculating the hash of a FIT image, the result is being stored in
an unaligned memory location. This causes problems (wrong hash) with the
fsl_hash CAAM engine on i.mx7ulp.

To fix the issue, this patch introduces a use of memalign() to allocate
memory for the hash value so that it is aligned correctly, similar to
what is done in file common/hash.c function hash_command() already.

Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
---
 boot/image-fit.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/boot/image-fit.c b/boot/image-fit.c
index 0de9180d2bb..c2283431925 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -24,6 +24,7 @@
 #include <mapmem.h>
 #include <asm/io.h>
 #include <malloc.h>
+#include <asm/cache.h>
 #include <asm/global_data.h>
 #ifdef CONFIG_DM_HASH
 #include <dm.h>
@@ -1263,12 +1264,17 @@ int calculate_hash(const void *data, int data_len, const char *name,
 static int fit_image_check_hash(const void *fit, int noffset, const void *data,
 				size_t size, char **err_msgp)
 {
+#ifdef USE_HOSTCC
 	uint8_t value[FIT_MAX_HASH_LEN];
+#else
+	u8 *value;
+#endif
 	int value_len;
 	const char *algo;
 	uint8_t *fit_value;
 	int fit_value_len;
 	int ignore;
+	int ret = 0;
 
 	*err_msgp = NULL;
 
@@ -1292,20 +1298,32 @@ static int fit_image_check_hash(const void *fit, int noffset, const void *data,
 		return -1;
 	}
 
+#ifndef USE_HOSTCC
+	value = memalign(ARCH_DMA_MINALIGN,
+			 sizeof(uint32_t) * HASH_MAX_DIGEST_SIZE);
+#endif
+
 	if (calculate_hash(data, size, algo, value, &value_len)) {
 		*err_msgp = "Unsupported hash algorithm";
-		return -1;
+		ret = -1;
+		goto hash_exit;
 	}
 
 	if (value_len != fit_value_len) {
 		*err_msgp = "Bad hash value len";
-		return -1;
+		ret = -1;
+		goto hash_exit;
 	} else if (memcmp(value, fit_value, value_len) != 0) {
 		*err_msgp = "Bad hash value";
-		return -1;
+		ret = -1;
+		goto hash_exit;
 	}
 
-	return 0;
+hash_exit:
+#ifndef USE_HOSTCC
+	unmap_sysmem(value);
+#endif
+	return ret;
 }
 
 int fit_image_verify_with_data(const void *fit, int image_noffset,

RE: [EXT] [PATCH] image-fit: aligned output of hash calculation

[Gaurav Jain]

Hi

> -----Original Message-----
> From: Christoph Fritz <chf.fritz@googlemail.com>
> Sent: Monday, March 20, 2023 1:19 PM
> To: u-boot <u-boot@lists.denx.de>
> Cc: Gaurav Jain <gaurav.jain@nxp.com>; Meenakshi Aggarwal
> <meenakshi.aggarwal@nxp.com>
> Subject: [EXT] [PATCH] image-fit: aligned output of hash calculation
> 
> Caution: EXT Email
> 
> When calculating the hash of a FIT image, the result is being stored in an
> unaligned memory location. This causes problems (wrong hash) with the
> fsl_hash CAAM engine on i.mx7ulp.
> 
> To fix the issue, this patch introduces a use of memalign() to allocate memory
> for the hash value so that it is aligned correctly, similar to what is done in file
> common/hash.c function hash_command() already.
> 
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> ---
>  boot/image-fit.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/boot/image-fit.c b/boot/image-fit.c index
> 0de9180d2bb..c2283431925 100644
> --- a/boot/image-fit.c
> +++ b/boot/image-fit.c
> @@ -24,6 +24,7 @@
>  #include <mapmem.h>
>  #include <asm/io.h>
>  #include <malloc.h>
> +#include <asm/cache.h>
>  #include <asm/global_data.h>
>  #ifdef CONFIG_DM_HASH
>  #include <dm.h>
> @@ -1263,12 +1264,17 @@ int calculate_hash(const void *data, int data_len,
> const char *name,  static int fit_image_check_hash(const void *fit, int noffset,
> const void *data,
>                                 size_t size, char **err_msgp)  {
> +#ifdef USE_HOSTCC
>         uint8_t value[FIT_MAX_HASH_LEN];
> +#else
> +       u8 *value;
> +#endif
>         int value_len;
>         const char *algo;
>         uint8_t *fit_value;
>         int fit_value_len;
>         int ignore;
> +       int ret = 0;
> 
>         *err_msgp = NULL;
> 
> @@ -1292,20 +1298,32 @@ static int fit_image_check_hash(const void *fit, int
> noffset, const void *data,
>                 return -1;
>         }
> 
> +#ifndef USE_HOSTCC
> +       value = memalign(ARCH_DMA_MINALIGN,
> +                        sizeof(uint32_t) * HASH_MAX_DIGEST_SIZE);
You can use same macro FIT_MAX_HASH_LEN.
> +#endif
> +
>         if (calculate_hash(data, size, algo, value, &value_len)) {
>                 *err_msgp = "Unsupported hash algorithm";
> -               return -1;
> +               ret = -1;
> +               goto hash_exit;
>         }
> 
>         if (value_len != fit_value_len) {
>                 *err_msgp = "Bad hash value len";
> -               return -1;
> +               ret = -1;
> +               goto hash_exit;
>         } else if (memcmp(value, fit_value, value_len) != 0) {
>                 *err_msgp = "Bad hash value";
> -               return -1;
> +               ret = -1;
> +               goto hash_exit;
>         }
> 
> -       return 0;
> +hash_exit:
> +#ifndef USE_HOSTCC
> +       unmap_sysmem(value);

I think free() is required??

Regards
Gaurav Jain

> +#endif
> +       return ret;
>  }
> 
>  int fit_image_verify_with_data(const void *fit, int image_noffset,

Re: [PATCH] image-fit: aligned output of hash calculation

[Simon Glass]

Hi Christopher,

On Mon, 20 Mar 2023 at 20:49, Christoph Fritz <chf.fritz@googlemail.com> wrote:
>
> When calculating the hash of a FIT image, the result is being stored in
> an unaligned memory location. This causes problems (wrong hash) with the
> fsl_hash CAAM engine on i.mx7ulp.
>
> To fix the issue, this patch introduces a use of memalign() to allocate
> memory for the hash value so that it is aligned correctly, similar to
> what is done in file common/hash.c function hash_command() already.
>
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> ---
>  boot/image-fit.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/boot/image-fit.c b/boot/image-fit.c
> index 0de9180d2bb..c2283431925 100644
> --- a/boot/image-fit.c
> +++ b/boot/image-fit.c
> @@ -24,6 +24,7 @@
>  #include <mapmem.h>
>  #include <asm/io.h>
>  #include <malloc.h>
> +#include <asm/cache.h>
>  #include <asm/global_data.h>
>  #ifdef CONFIG_DM_HASH
>  #include <dm.h>
> @@ -1263,12 +1264,17 @@ int calculate_hash(const void *data, int data_len, const char *name,
>  static int fit_image_check_hash(const void *fit, int noffset, const void *data,
>                                 size_t size, char **err_msgp)
>  {
> +#ifdef USE_HOSTCC
>         uint8_t value[FIT_MAX_HASH_LEN];
> +#else
> +       u8 *value;
> +#endif

Could you use tools_build() instead of an #ifdef, e.g.

         u8 value_s[FIT_MAX_HASH_LEN], value = value_s;

if (!tools_build())
    value = memalign(ARCH_DMA_MINALIGN,
                          sizeof(uint32_t) * HASH_MAX_DIGEST_SIZE);

>         uint8_t value[FIT_MAX_HASH_LEN];
>         int value_len;
>         const char *algo;
>         uint8_t *fit_value;
>         int fit_value_len;
>         int ignore;
> +       int ret = 0;
>
>         *err_msgp = NULL;
>
> @@ -1292,20 +1298,32 @@ static int fit_image_check_hash(const void *fit, int noffset, const void *data,
>                 return -1;
>         }
>
> +#ifndef USE_HOSTCC
> +       value = memalign(ARCH_DMA_MINALIGN,
> +                        sizeof(uint32_t) * HASH_MAX_DIGEST_SIZE);
> +#endif
> +
>         if (calculate_hash(data, size, algo, value, &value_len)) {
>                 *err_msgp = "Unsupported hash algorithm";
> -               return -1;
> +               ret = -1;
> +               goto hash_exit;
>         }
>
>         if (value_len != fit_value_len) {
>                 *err_msgp = "Bad hash value len";
> -               return -1;
> +               ret = -1;
> +               goto hash_exit;
>         } else if (memcmp(value, fit_value, value_len) != 0) {
>                 *err_msgp = "Bad hash value";
> -               return -1;
> +               ret = -1;
> +               goto hash_exit;
>         }
>
> -       return 0;
> +hash_exit:
> +#ifndef USE_HOSTCC
> +       unmap_sysmem(value);
> +#endif
> +       return ret;
>  }
>
>  int fit_image_verify_with_data(const void *fit, int image_noffset,
>

Regards,
Simon