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

[Philip Oberfichtner]

Hi everybody,

On Mon, 2021-12-06 at 08:22 +0000, Ye Li wrote:
> 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.
> > 

I have the same problem on my imx6d. This patch works fine for me.


> > Signed-off-by: Christian Sørensen <yocto at 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

Is this patch still active? The change request is already two months
old. If the original patch request is abandoned, should I contribute a
[PATCH v2]?

Best Regards,
Philip Oberfichtner