[PATCH] crypto: fsl_hash: fixup alignment if necessary

[Fabio Estevam]

Ye Li, Peng, Marek

Could you please help review this patch?

On Thu, Dec 19, 2024 at 3:33 PM Christoph Fritz
<chf.fritz at googlemail.com> wrote:
>
> Trying to boot a fitImage on an i.mx7ulp results in alignement issue:
>
>    ## Checking hash(es) for FIT Image at 90000000 ...
>       Hash(es) for Image 0 (kernel-1): sha256
>    CACHE: Misaligned operation at range [90000108, 906d9b08]
>    CACHE: Misaligned operation at range [9f6349f0, 9f634a30]
>    CACHE: Misaligned operation at range [9f6349f0, 9f634a30]
>    CACHE: Misaligned operation at range [9f6349f0, 9f634a30]
>    CACHE: Misaligned operation at range [9f6349f0, 9f634a30]
>     error!
>    Bad hash value for 'hash-1' hash node in 'kernel-1' image node
>    Bad hash in FIT image!
>
> Commit 41b2182af73e ("crypto: fsl_hash: Remove unnecessary alignment
> check in caam_hash()") removed a specific check which is not necessary
> for a Layerscape LX2160A but would have catched this directly in
> caam_hash().
>
> The actual root cause is currently unknown, so fix up input and output
> buffers if not aligned.
>
> Signed-off-by: Christoph Fritz <chf.fritz at googlemail.com>
> ---
>  drivers/crypto/fsl/fsl_hash.c | 87 +++++++++++++++++++++++++++++++----
>  1 file changed, 78 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/crypto/fsl/fsl_hash.c b/drivers/crypto/fsl/fsl_hash.c
> index 79b32e2627c..ad8b5640f6f 100644
> --- a/drivers/crypto/fsl/fsl_hash.c
> +++ b/drivers/crypto/fsl/fsl_hash.c
> @@ -178,17 +178,78 @@ static int caam_hash_finish(void *hash_ctx, void *dest_buf,
>         return ret;
>  }
>
> +static int fixup_input_buffer(const unsigned char **ppbuf, unsigned int len)
> +{
> +       unsigned int aligned_size = ALIGN(len, ARCH_DMA_MINALIGN);
> +       unsigned char *aligned_buf = malloc_cache_aligned(aligned_size);
> +
> +       if (!aligned_buf) {
> +               debug("Not enough memory for aligned input buffer\n");
> +               return -ENOMEM;
> +       }
> +
> +       memcpy(aligned_buf, *ppbuf, len);
> +
> +       if (aligned_size > len)
> +               memset(aligned_buf + len, 0, aligned_size - len);
> +
> +       *ppbuf = aligned_buf;
> +       return 0;
> +}
> +
> +static int fixup_output_buffer(unsigned char **ppbuf, unsigned int len)
> +{
> +       unsigned int aligned_size = ALIGN(len, ARCH_DMA_MINALIGN);
> +       unsigned char *aligned_buf = malloc_cache_aligned(aligned_size);
> +
> +       if (!aligned_buf) {
> +               debug("Not enough memory for aligned output buffer\n");
> +               return -ENOMEM;
> +       }
> +
> +       memset(aligned_buf, 0, aligned_size);
> +
> +       *ppbuf = aligned_buf;
> +       return 0;
> +}
> +
>  int caam_hash(const unsigned char *pbuf, unsigned int buf_len,
>               unsigned char *pout, enum caam_hash_algos algo)
>  {
> -       int ret = 0;
> -       uint32_t *desc;
> +       unsigned int digest_size = driver_hash[algo].digestsize;
> +       bool pbuf_fixed = false, pout_fixed = false;
> +       const unsigned char *orig_pbuf = pbuf;
> +       unsigned char *orig_pout = pout;
>         unsigned int size;
> +       int ret = 0;
> +       u32 *desc;
> +
> +       /* fix up input buffer if needed */
> +       if (!IS_ALIGNED((uintptr_t)pbuf, ARCH_DMA_MINALIGN)) {
> +               debug("Fix up pbuf Address alignment\n");
> +               ret = fixup_input_buffer(&pbuf, buf_len);
> +               if (ret)
> +                       return ret;
> +               pbuf_fixed = true;
> +       }
> +
> +       /* fix up output buffer if needed */
> +       if (!IS_ALIGNED((uintptr_t)pout, ARCH_DMA_MINALIGN)) {
> +               debug("Fix up pout Address alignment\n");
> +               ret = fixup_output_buffer(&pout, digest_size);
> +               if (ret) {
> +                       if (pbuf_fixed)
> +                               free((void *)pbuf);
> +                       return ret;
> +               }
> +               pout_fixed = true;
> +       }
>
>         desc = malloc_cache_aligned(sizeof(int) * MAX_CAAM_DESCSIZE);
>         if (!desc) {
>                 debug("Not enough memory for descriptor allocation\n");
> -               return -ENOMEM;
> +               ret = -ENOMEM;
> +               goto out;
>         }
>
>         size = ALIGN(buf_len, ARCH_DMA_MINALIGN);
> @@ -196,21 +257,29 @@ int caam_hash(const unsigned char *pbuf, unsigned int buf_len,
>
>         inline_cnstr_jobdesc_hash(desc, pbuf, buf_len, pout,
>                                   driver_hash[algo].alg_type,
> -                                 driver_hash[algo].digestsize,
> -                                 0);
> +                                 digest_size, 0);
>
>         size = ALIGN(sizeof(int) * MAX_CAAM_DESCSIZE, ARCH_DMA_MINALIGN);
>         flush_dcache_range((unsigned long)desc, (unsigned long)desc + size);
> -       size = ALIGN(driver_hash[algo].digestsize, ARCH_DMA_MINALIGN);
> +
> +       size = ALIGN(digest_size, ARCH_DMA_MINALIGN);
>         invalidate_dcache_range((unsigned long)pout, (unsigned long)pout + size);
>
>         ret = run_descriptor_jr(desc);
>
> -       size = ALIGN(driver_hash[algo].digestsize, ARCH_DMA_MINALIGN);
> -       invalidate_dcache_range((unsigned long)pout,
> -                               (unsigned long)pout + size);
> +       invalidate_dcache_range((unsigned long)pout, (unsigned long)pout + size);
> +
> +       if (ret == 0 && pout_fixed)
> +               memcpy(orig_pout, pout, digest_size);
>
>         free(desc);
> +
> +out:
> +       if (pbuf_fixed)
> +               free((void *)pbuf);
> +       if (pout_fixed)
> +               free((void *)pout);
> +
>         return ret;
>  }
>
> --
> 2.39.5
>
>