Message ID | 8945e17ea15cf0bf9fedd4fb7f9820f30981f3bf.camel@googlemail.com |
---|---|
State | Changes Requested |
Delegated to: | Simon Glass |
Headers | show |
Series | image-fit: aligned output of hash calculation | expand |
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,
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
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,
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(-)