* [PATCH v3 1/2] crypto: caam - Fix wrong unmap operations
2026-10-20 2:54 [PATCH v3 0/2] crypto: caam - Fix RSA error-path unmap and unaligned dst buffers Changwei Zou
@ 2026-10-20 2:54 ` Changwei Zou
2026-10-20 2:54 ` [PATCH v3 2/2] crypto: caam - Use bounce buffer for unaligned RSA destination buffers Changwei Zou
1 sibling, 0 replies; 4+ messages in thread
From: Changwei Zou @ 2026-10-20 2:54 UTC (permalink / raw)
To: herbert, sahil.malhotra
Cc: davem, horia.geanta, pankaj.gupta, gaurav.jain, jeffbarnes,
stephane.debusne, iuliana.prodan, martin.kepplinger-novakovic,
lukas, sahil.malhotra, changwei.zou, linux-crypto, linux-kernel
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 <changwei.zou@canonical.com>
Assisted-by: OpenCode:claude-sonnet-4.6
Reviewed-by: Sahil Malhotra <sahil.malhotra@nxp.com>
---
drivers/crypto/caam/caampkc.c | 43 +++++++++++++++++++++++++----------
1 file changed, 31 insertions(+), 12 deletions(-)
diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index cb001aa1de66..18a728849147 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -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;
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v3 2/2] crypto: caam - Use bounce buffer for unaligned RSA destination buffers
2026-10-20 2:54 [PATCH v3 0/2] crypto: caam - Fix RSA error-path unmap and unaligned dst buffers Changwei Zou
2026-10-20 2:54 ` [PATCH v3 1/2] crypto: caam - Fix wrong unmap operations Changwei Zou
@ 2026-10-20 2:54 ` Changwei Zou
2026-11-11 10:50 ` Herbert Xu
1 sibling, 1 reply; 4+ messages in thread
From: Changwei Zou @ 2026-10-20 2:54 UTC (permalink / raw)
To: herbert, sahil.malhotra
Cc: davem, horia.geanta, pankaj.gupta, gaurav.jain, jeffbarnes,
stephane.debusne, iuliana.prodan, martin.kepplinger-novakovic,
lukas, sahil.malhotra, changwei.zou, linux-crypto, linux-kernel,
Martin Kepplinger-Novakovic
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 <Martin.Kepplinger-Novakovic@ginzinger.com>
Closes: https://lore.kernel.org/r/6029acc0f0ddfe25e2537c2866d54fd7f54bc182.camel@ginzinger.com
Signed-off-by: Changwei Zou <changwei.zou@canonical.com>
Assisted-by: OpenCode:claude-sonnet-4.6
Tested-by: Martin Kepplinger-Novakovic <martin.kepplinger-novakovic@ginzinger.com>
Tested-by: Stephane Debusne <stephane.debusne@cynove.com>
---
drivers/crypto/caam/caampkc.c | 87 ++++++++++++++++++++++++++++++++++-
drivers/crypto/caam/caampkc.h | 6 +++
2 files changed, 92 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index 18a728849147..aa782978b1e6 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -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;
}
diff --git a/drivers/crypto/caam/caampkc.h b/drivers/crypto/caam/caampkc.h
index 96d03704c9be..efad91d6058f 100644
--- a/drivers/crypto/caam/caampkc.h
+++ b/drivers/crypto/caam/caampkc.h
@@ -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);
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread