Re: [EXT] [PATCH] crypto/fsl: Fallback to SW sha1/256 is misaligned buffers

[Ye Li <ye.li@nxp.com>]

On Fri, 2021-11-05 at 10:42 +0100, Christian Sørensen wrote:
> Caution: EXT Email
> 
> Problem:
> With U-Boot 2021.10, we currently cannot load a fitImage on our imx7d
> board, due to misaligned buffers.
> 
> Reason:
> Commit 92055e138f28 ("image: Drop if/elseif hash selection in
> calculate_hash()")
> changed the way the FIT were verified. Previously, SW sha1/256 were
> always
> used. Due to that commit, that can now be done in hardware.
> caam_hash requires both the input, pbuf, and output buffer, pout, to
> be
> aligned. E.g. for the kernel data, pbuf will be the data start
> address
> for the kernel. The data start address is dependent on how the FIT is
> constructed and what address the FIT is loaded to. I.e.; it is fairly
> likely that we have a case of pbuf to not be aligned. pout is even
> more
> likely to not be aligned since it is simply a stack variable declared
> in
> fit_image_check_hash in common/image-fit.c.
> So to rely upon both of these buffers to be aligned, makes errors
> fairly
> likely.
> 
> Solution:
> I wont propose copying the entire input buffer due to its size, so
> instead
> just fallback to use the sw sha1/sha256 if buffers is misaligned.
> 
> Signed-off-by: Christian Sørensen <yocto@bsorensen.net>
> ---
> 
>  drivers/crypto/fsl/fsl_hash.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/fsl/fsl_hash.c
> b/drivers/crypto/fsl/fsl_hash.c
> index 8b5c26db07..48dd10883e 100644
> --- a/drivers/crypto/fsl/fsl_hash.c
> +++ b/drivers/crypto/fsl/fsl_hash.c
> @@ -16,6 +16,9 @@
>  #include <hw_sha.h>
>  #include <asm/cache.h>
>  #include <linux/errno.h>
> +#include <u-boot/sha1.h>
> +#include <u-boot/sha256.h>
> +#include <image.h>
> 
>  #define CRYPTO_MAX_ALG_NAME    80
>  #define SHA1_DIGEST_SIZE        20
> @@ -176,8 +179,12 @@ int caam_hash(const unsigned char *pbuf,
> unsigned int buf_len,
> 
>         if (!IS_ALIGNED((uintptr_t)pbuf, ARCH_DMA_MINALIGN) ||
>             !IS_ALIGNED((uintptr_t)pout, ARCH_DMA_MINALIGN)) {
> -               puts("Error: Address arguments are not aligned\n");
> -               return -EINVAL;
> +               printf("Fallback to SW hash due to misaligned
> buffers\n");
> +               if (algo == SHA1)
> +                       sha1_csum_wd(pbuf, buf_len, pout,
> CHUNKSZ_SHA1);
> +               else
> +                       sha256_csum_wd(pbuf, buf_len, pout,
> CHUNKSZ_SHA256);
> +               return 0;

How about adding “#ifdef CONFIG_SHA1” and “#ifdef CONFIG_SHA256” here
?  Then it can depend on users’ selection to determine the fallback

Best regards,
Ye Li

>         }
> 
>         size = ALIGN(buf_len, ARCH_DMA_MINALIGN);
> --
> 2.25.1
>