Skip to contentSkip to conversation

crypto: caam - Fix RSA error-path unmap and unaligned dst buffers

PATCH In review Changwei Zou 2 participants 4 messages Oct 20 – Nov 11

Subsystems (2)
Crypto API Freescale CAAM (Cryptographic Acceleration and Assurance Module) driver
lore
Share & tools
lore

Changwei Zou

author
#0 lore raw

v3 cover crypto: caam - Fix RSA error-path unmap and unaligned dst buffers

This series fixes two issues in the rsa-caam driver.

Patch 1 fixes wrong unmap operations where rsa_pub_unmap() was called regardless of the key type.

Patch 2 adds a bounce buffer to handle unaligned RSA destination buffers on DMA-incoherent hardware such as i.MX8, fixing intermittent -EKEYREJECTED errors when loading signed kernel modules.

Changes in v3:
- Patch 1: Dispatch the error-path unmap on the operation (the completion callback rsa_pub_done vs rsa_priv_f_done) instead of key->d, so the unmap always matches the map.
- Patch 2: Bound the bounce-buffer copy-back to the RSA output size (key->n_sz) instead of the full dst_len, so the trailing capacity of the caller's destination buffer is left untouched; add Tested-by from Stephane Debusne.

Changes in v2:
- Patch 1: Add Fixes: tag; pick up Reviewed-by from Sahil Malhotra.
- Patch 2: Move the !bounce_buf check out of do_rsa_bounce_buf() into the caller rsa_bounce_buf_done() so the helper is only entered when a bounce buffer actually exists; add Reported-by/Closes and Tested-by from Martin Kepplinger-Novakovic.

Link to v2:
https://lore.kernel.org/linux-crypto/[email protected]/

Link to v1:
https://lore.kernel.org/linux-crypto/[email protected]/

Changwei Zou (2):
crypto: caam - Fix wrong unmap operations
crypto: caam - Use bounce buffer for unaligned RSA destination buffers

 drivers/crypto/caam/caampkc.c | 130 ++++++++++++++++++++++++++++++----
 drivers/crypto/caam/caampkc.h |   6 ++
 2 files changed, 123 insertions(+), 13 deletions(-)
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH v3 1/2 crypto: caam - Fix wrong unmap operations

Since priv_form only takes values FORM1/FORM2/FORM3 with no distinct public key enumerator, rsa_pub_unmap() was not called correctly in akcipher_do_one_req() and akcipher_enqueue_req().

Use the completion callback (rsa_pub_done vs rsa_priv_f_done) to distinguish public from private key operations, then dispatch to the correct unmap function based on key->priv_form.

caam_rsa_set_priv_key_form() implicitly relies on zero-initialization for priv_form. Set priv_form = FORM1 explicitly at the head of the function for clarity and robustness.

Fixes bf53795025a2 ("crypto: caam - add crypto_engine support for RSA algorithms")
Signed-off-by Changwei Zou <[email protected]>
Assisted-by OpenCode:claude-sonnet-4.6
Reviewed-by Sahil Malhotra <[email protected]>
---
 drivers/crypto/caam/caampkc.c | 43 +++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 12 deletions(-)
drivers/crypto/caam/caampkc.c +31 -12
index cb001aa1de66..18a728849147 100644
@@ -379,6 +379,7 @@ static int akcipher_do_one_req(struct crypto_engine *engine, void *areq)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct caam_rsa_req_ctx *req_ctx = akcipher_request_ctx(req);
 	struct caam_rsa_ctx *ctx = akcipher_tfm_ctx_dma(tfm);
+	struct caam_rsa_key *key = &ctx->key;
 	struct device *jrdev = ctx->dev;
 	u32 *desc = req_ctx->edesc->hw_desc;
 	int ret;

@@ -391,7 +392,21 @@ static int akcipher_do_one_req(struct crypto_engine *engine, void *areq)
 		return ret;
 
 	if (ret != -EINPROGRESS) {
-		rsa_pub_unmap(jrdev, req_ctx->edesc, req);
+		if (req_ctx->akcipher_op_done == rsa_pub_done) {
+			rsa_pub_unmap(jrdev, req_ctx->edesc, req);
+		} else {
+			switch (key->priv_form) {
+			case FORM1:
+				rsa_priv_f1_unmap(jrdev, req_ctx->edesc, req);
+				break;
+			case FORM2:
+				rsa_priv_f2_unmap(jrdev, req_ctx->edesc, req);
+				break;
+			case FORM3:
+				rsa_priv_f3_unmap(jrdev, req_ctx->edesc, req);
+				break;
+			}
+		}
 		rsa_io_unmap(jrdev, req_ctx->edesc, req);
 		kfree(req_ctx->edesc);
 	} else {

@@ -691,18 +706,20 @@ static int akcipher_enqueue_req(struct device *jrdev,
 		ret = caam_jr_enqueue(jrdev, desc, cbk, req);
 
 	if ((ret != -EINPROGRESS) && (ret != -EBUSY)) {
-		switch (key->priv_form) {
-		case FORM1:
-			rsa_priv_f1_unmap(jrdev, edesc, req);
-			break;
-		case FORM2:
-			rsa_priv_f2_unmap(jrdev, edesc, req);
-			break;
-		case FORM3:
-			rsa_priv_f3_unmap(jrdev, edesc, req);
-			break;
-		default:
+		if (cbk == rsa_pub_done) {
 			rsa_pub_unmap(jrdev, edesc, req);
+		} else {
+			switch (key->priv_form) {
+			case FORM1:
+				rsa_priv_f1_unmap(jrdev, edesc, req);
+				break;
+			case FORM2:
+				rsa_priv_f2_unmap(jrdev, edesc, req);
+				break;
+			case FORM3:
+				rsa_priv_f3_unmap(jrdev, edesc, req);
+				break;
+			}
 		}
 		rsa_io_unmap(jrdev, edesc, req);
 		kfree(edesc);

@@ -992,6 +1009,8 @@ static int caam_rsa_set_priv_key_form(struct caam_rsa_ctx *ctx,
 	size_t q_sz = raw_key->q_sz;
 	unsigned aligned_size;
 
+	rsa_key->priv_form = FORM1;
+
 	rsa_key->p = caam_read_raw_data(raw_key->p, &p_sz);
 	if (!rsa_key->p)
 		return -ENOMEM;
Signature · 1 line
2.43.0
author
#2 lore raw

PATCH v3 2/2 crypto: caam - Use bounce buffer for unaligned RSA destination buffers

In reply to Changwei Zou

The rsa-caam driver directly DMA-maps the destination buffer supplied by the caller via req->dst without checking whether it meets the cacheline alignment requirements of DMA-incoherent hardware such as i.MX8.

On CPUs with non-coherent DMA caches, if the destination buffer shares a cacheline with other data (i.e. it is not cacheline-aligned), cache writeback/invalidation during DMA can corrupt adjacent memory or cause stale data to be read back. This manifests as intermittent -EKEYREJECTED errors when loading signed kernel modules.

When any segment of req->dst is not cacheline-aligned in either its start offset or length, allocate a single contiguous aligned bounce buffer covering the full dst_len rounded up to a cacheline multiple, redirect the operation to it, and copy the result back to the original destination once the hardware has completed successfully.

Introduce a helper sg_is_dma_aligned() that checks both sg->offset and sg->length for cacheline alignment. Checking sg->offset suffices for the start address since physical pages are always page-aligned and PAGE_SIZE is a multiple of dma_get_cache_alignment(). sg->length is also checked to ensure the buffer end does not share a cacheline with adjacent memory.

The intermittent error 'Key was rejected by service' on i.MX8 with CAAM can be triggered when loading signed kernel modules:

    for i in $(seq 1 100); do
        sudo modprobe xfs 2>&1 && echo "SUCCESS on attempt $i" \
        && sudo rmmod xfs || echo "FAILED on attempt $i"
    done
Reported-by Martin Kepplinger-Novakovic <[email protected]>
Closes https://lore.kernel.org/r/[email protected] open here
Signed-off-by Changwei Zou <[email protected]>
Assisted-by OpenCode:claude-sonnet-4.6
Tested-by Martin Kepplinger-Novakovic <[email protected]>
Tested-by Stephane Debusne <[email protected]>
---
 drivers/crypto/caam/caampkc.c | 87 ++++++++++++++++++++++++++++++++++-
 drivers/crypto/caam/caampkc.h |  6 +++
 2 files changed, 92 insertions(+), 1 deletion(-)
drivers/crypto/caam/caampkc.c +86 -1
index 18a728849147..aa782978b1e6 100644
@@ -59,6 +59,50 @@ static void rsa_io_unmap(struct device *dev, struct rsa_edesc *edesc,
 				 DMA_TO_DEVICE);
 }
 
+static int do_rsa_bounce_buf(struct akcipher_request *req,
+			     struct caam_rsa_req_ctx *req_ctx, int req_err)
+{
+	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
+	struct caam_rsa_ctx *ctx = akcipher_tfm_ctx_dma(tfm);
+	struct caam_rsa_key *key = &ctx->key;
+	int nents, err = 0;
+
+	/*
+	 * Only copy back to the original destination on success, and only the
+	 * actual RSA output (key->n_sz bytes) rather than the full dst_len, so
+	 * the trailing capacity of the caller's buffer is left untouched.
+	 */
+	if (!req_err) {
+		nents = sg_nents_for_len(req_ctx->orig_dst, key->n_sz);
+		if (nents < 0)
+			err = nents;
+		else if (sg_copy_from_buffer(req_ctx->orig_dst, nents,
+					     req_ctx->bounce_buf,
+					     key->n_sz) != key->n_sz)
+			err = -EFAULT;
+	}
+
+	kfree(req_ctx->bounce_buf);
+	req_ctx->bounce_buf = NULL;
+	req->dst = req_ctx->orig_dst;
+
+	return err;
+}
+
+static inline void rsa_bounce_buf_done(struct akcipher_request *req, int *err)
+{
+	struct caam_rsa_req_ctx *req_ctx = akcipher_request_ctx(req);
+	int cperr;
+
+	if (!req_ctx->bounce_buf)
+		return;
+
+	cperr = do_rsa_bounce_buf(req, req_ctx, *err);
+
+	if (!*err)
+		*err = cperr;
+}
+
 static void rsa_pub_unmap(struct device *dev, struct rsa_edesc *edesc,
 			  struct akcipher_request *req)
 {

@@ -138,6 +182,7 @@ static void rsa_pub_done(struct device *dev, u32 *desc, u32 err, void *context)
 	rsa_pub_unmap(dev, edesc, req);
 	rsa_io_unmap(dev, edesc, req);
 	kfree(edesc);
+	rsa_bounce_buf_done(req, &ecode);
 
 	/*
 	 * If no backlog flag, the completion of the request is done

@@ -181,6 +226,7 @@ static void rsa_priv_f_done(struct device *dev, u32 *desc, u32 err,
 
 	rsa_io_unmap(dev, edesc, req);
 	kfree(edesc);
+	rsa_bounce_buf_done(req, &ecode);
 
 	/*
 	 * If no backlog flag, the completion of the request is done

@@ -246,6 +292,12 @@ static int caam_rsa_count_leading_zeros(struct scatterlist *sgl,
 	return tbytes - nbytes;
 }
 
+static inline bool sg_is_dma_aligned(struct scatterlist *sg)
+{
+	return IS_ALIGNED(sg->offset, dma_get_cache_alignment()) &&
+	       IS_ALIGNED(sg->length, dma_get_cache_alignment());
+}
+
 static struct rsa_edesc *rsa_edesc_alloc(struct akcipher_request *req,
 					 size_t desclen)
 {

@@ -291,11 +343,34 @@ static struct rsa_edesc *rsa_edesc_alloc(struct akcipher_request *req,
 				     req_ctx->fixup_src_len);
 	dst_nents = sg_nents_for_len(req->dst, req->dst_len);
 
+	req_ctx->bounce_buf = NULL;
+	req_ctx->orig_dst = req->dst;
+	if (req->dst_len > 0) {
+		struct scatterlist *sg;
+		int i;
+
+		for_each_sg(req->dst, sg, dst_nents, i) {
+			if (!sg_is_dma_aligned(sg)) {
+				req_ctx->bounce_buf =
+					kzalloc(ALIGN(req->dst_len,
+						      dma_get_cache_alignment()),
+						flags);
+				if (!req_ctx->bounce_buf)
+					return ERR_PTR(-ENOMEM);
+				sg_init_one(&req_ctx->dst, req_ctx->bounce_buf,
+					    req->dst_len);
+				req->dst = &req_ctx->dst;
+				dst_nents = 1;
+				break;
+			}
+		}
+	}
+
 	mapped_src_nents = dma_map_sg(dev, req_ctx->fixup_src, src_nents,
 				      DMA_TO_DEVICE);
 	if (unlikely(!mapped_src_nents)) {
 		dev_err(dev, "unable to map source\n");
-		return ERR_PTR(-ENOMEM);
+		goto bounce_fail;
 	}
 	mapped_dst_nents = dma_map_sg(dev, req->dst, dst_nents,
 				      DMA_FROM_DEVICE);

@@ -368,6 +443,10 @@ static struct rsa_edesc *rsa_edesc_alloc(struct akcipher_request *req,
 	dma_unmap_sg(dev, req->dst, dst_nents, DMA_FROM_DEVICE);
 src_fail:
 	dma_unmap_sg(dev, req_ctx->fixup_src, src_nents, DMA_TO_DEVICE);
+bounce_fail:
+	kfree(req_ctx->bounce_buf);
+	req_ctx->bounce_buf = NULL;
+	req->dst = req_ctx->orig_dst;
 	return ERR_PTR(-ENOMEM);
 }
 

@@ -409,6 +488,7 @@ static int akcipher_do_one_req(struct crypto_engine *engine, void *areq)
 		}
 		rsa_io_unmap(jrdev, req_ctx->edesc, req);
 		kfree(req_ctx->edesc);
+		rsa_bounce_buf_done(req, &ret);
 	} else {
 		ret = 0;
 	}

@@ -723,6 +803,7 @@ static int akcipher_enqueue_req(struct device *jrdev,
 		}
 		rsa_io_unmap(jrdev, edesc, req);
 		kfree(edesc);
+		rsa_bounce_buf_done(req, &ret);
 	}
 
 	return ret;

@@ -764,6 +845,7 @@ static int caam_rsa_enc(struct akcipher_request *req)
 init_fail:
 	rsa_io_unmap(jrdev, edesc, req);
 	kfree(edesc);
+	rsa_bounce_buf_done(req, &ret);
 	return ret;
 }
 

@@ -793,6 +875,7 @@ static int caam_rsa_dec_priv_f1(struct akcipher_request *req)
 init_fail:
 	rsa_io_unmap(jrdev, edesc, req);
 	kfree(edesc);
+	rsa_bounce_buf_done(req, &ret);
 	return ret;
 }
 

@@ -822,6 +905,7 @@ static int caam_rsa_dec_priv_f2(struct akcipher_request *req)
 init_fail:
 	rsa_io_unmap(jrdev, edesc, req);
 	kfree(edesc);
+	rsa_bounce_buf_done(req, &ret);
 	return ret;
 }
 

@@ -851,6 +935,7 @@ static int caam_rsa_dec_priv_f3(struct akcipher_request *req)
 init_fail:
 	rsa_io_unmap(jrdev, edesc, req);
 	kfree(edesc);
+	rsa_bounce_buf_done(req, &ret);
 	return ret;
 }
drivers/crypto/caam/caampkc.h +6 -0
index 96d03704c9be..efad91d6058f 100644
@@ -103,6 +103,9 @@ struct caam_rsa_ctx {
  * @src           : input scatterlist (stripped of leading zeros)
  * @fixup_src     : input scatterlist (that might be stripped of leading zeros)
  * @fixup_src_len : length of the fixup_src input scatterlist
+ * @dst           : destination scatterlist backed by bounce buffer (if needed)
+ * @bounce_buf    : DMA-aligned bounce buffer for destination (or NULL)
+ * @orig_dst      : original destination scatterlist (before bounce substitution)
  * @edesc         : s/w-extended rsa descriptor
  * @akcipher_op_done : callback used when operation is done
  */

@@ -110,6 +113,9 @@ struct caam_rsa_req_ctx {
 	struct scatterlist src[2];
 	struct scatterlist *fixup_src;
 	unsigned int fixup_src_len;
+	struct scatterlist dst;
+	u8 *bounce_buf;
+	struct scatterlist *orig_dst;
 	struct rsa_edesc *edesc;
 	void (*akcipher_op_done)(struct device *jrdev, u32 *desc, u32 err,
 				 void *context);
Signature · 1 line
2.43.0
maintainer
#3 lore raw

Re: [PATCH v3 2/2] crypto: caam - Use bounce buffer for unaligned RSA destination buffers

Quoted 37 lines · Changwei Zou

The rsa-caam driver directly DMA-maps the destination buffer supplied by the caller via req->dst without checking whether it meets the cacheline alignment requirements of DMA-incoherent hardware such as i.MX8.

On CPUs with non-coherent DMA caches, if the destination buffer shares a cacheline with other data (i.e. it is not cacheline-aligned), cache writeback/invalidation during DMA can corrupt adjacent memory or cause stale data to be read back. This manifests as intermittent -EKEYREJECTED errors when loading signed kernel modules.

When any segment of req->dst is not cacheline-aligned in either its start offset or length, allocate a single contiguous aligned bounce buffer covering the full dst_len rounded up to a cacheline multiple, redirect the operation to it, and copy the result back to the original destination once the hardware has completed successfully.

Introduce a helper sg_is_dma_aligned() that checks both sg->offset and sg->length for cacheline alignment. Checking sg->offset suffices for the start address since physical pages are always page-aligned and PAGE_SIZE is a multiple of dma_get_cache_alignment(). sg->length is also checked to ensure the buffer end does not share a cacheline with adjacent memory.

The intermittent error 'Key was rejected by service' on i.MX8 with CAAM can be triggered when loading signed kernel modules:

    for i in $(seq 1 100); do
        sudo modprobe xfs 2>&1 && echo "SUCCESS on attempt $i" \
        && sudo rmmod xfs || echo "FAILED on attempt $i"
    done
Reported-by Martin Kepplinger-Novakovic <[email protected]>
Closes https://lore.kernel.org/r/[email protected] open here
Signed-off-by Changwei Zou <[email protected]>
Assisted-by OpenCode:claude-sonnet-4.6
Tested-by Martin Kepplinger-Novakovic <[email protected]>
Tested-by Stephane Debusne <[email protected]>
Quoted 4 lines
---
 drivers/crypto/caam/caampkc.c | 87 ++++++++++++++++++++++++++++++++++-
 drivers/crypto/caam/caampkc.h |  6 +++
 2 files changed, 92 insertions(+), 1 deletion(-)

Please respond to the Sashiko comments:

https://sashiko.dev/#/patchset/20261020025426.955531-1-changwei.zou%40canonical.com

I'm also disappointed with the lack of response from the caam maintainers. Unless active maintainence can be demonstrated for this driver, I'm going to have it removed again.

Thanks,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt