Re: [PATCH] crypto: fsl_hash: fixup alignment if necessary

Ye Li, Peng, Marek

Could you please help review this patch?

On Thu, Dec 19, 2024 at 3:33 PM Christoph Fritz chf.fritz@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@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