mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/3] crypto: caam: Fix DMA mapping leak in the cbc(paes) job path
@ 2026-07-26  8:15 Richard Weinberger
  2026-07-26  8:15 ` [PATCH 2/3] crypto: caam: Map the paes protected key once per tfm Richard Weinberger
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Richard Weinberger @ 2026-07-26  8:15 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-crypto, davem, herbert, gaurav.jain, pankaj.gupta,
	horia.geanta, ebiggers, upstream+linux, Richard Weinberger

For a protected key, init_skcipher_job() maps the second job descriptor
so that the blob decapsulation descriptor can jump to it, but keeps the
address in a local variable. Nothing can ever unmap it, and the buffer
it covers lives in edesc, which skcipher_crypt_done() frees while the
mapping is still live. The mapping error is not checked either.

cbc(paes) is reachable through AF_ALG, so an unprivileged process leaks
one mapping per request, exhausting IOVA space or swiotlb buffers.

Store the mapping in the extended descriptor, release it in
skcipher_unmap() and propagate mapping failures to the caller.

Assisted-by: Claude-Code:claude-fable-5
Reported-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/crypto/caam/caamalg.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index ddbd60cf3..c83d28509 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -983,6 +983,9 @@ struct aead_edesc {
  * @sec4_sg_bytes: length of dma mapped sec4_sg space
  * @bklog: stored to determine if the request needs backlog
  * @sec4_sg_dma: bus physical mapped address of h/w link table
+ * @desc_dma: bus physical mapped address of the cipher job descriptor the
+ *	      blob decapsulation descriptor jumps to (protected key only)
+ * @desc_dma_bytes: length of the mapping at @desc_dma
  * @sec4_sg: pointer to h/w link table
  * @hw_desc: the h/w job descriptor followed by any referenced link tables
  *	     and IV
@@ -996,6 +999,8 @@ struct skcipher_edesc {
 	int sec4_sg_bytes;
 	bool bklog;
 	dma_addr_t sec4_sg_dma;
+	dma_addr_t desc_dma;
+	unsigned int desc_dma_bytes;
 	struct sec4_sg_entry *sec4_sg;
 	u32 hw_desc[];
 };
@@ -1037,6 +1042,10 @@ static void skcipher_unmap(struct device *dev, struct skcipher_edesc *edesc,
 	struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
 	int ivsize = crypto_skcipher_ivsize(skcipher);
 
+	if (edesc->desc_dma)
+		dma_unmap_single(dev, edesc->desc_dma, edesc->desc_dma_bytes,
+				 DMA_TO_DEVICE);
+
 	caam_unmap(dev, req->src, req->dst,
 		   edesc->src_nents, edesc->dst_nents,
 		   edesc->iv_dma, ivsize,
@@ -1313,9 +1322,9 @@ static void init_authenc_job(struct aead_request *req,
 /*
  * Fill in skcipher job descriptor
  */
-static void init_skcipher_job(struct skcipher_request *req,
-			      struct skcipher_edesc *edesc,
-			      const bool encrypt)
+static int init_skcipher_job(struct skcipher_request *req,
+			     struct skcipher_edesc *edesc,
+			     const bool encrypt)
 {
 	struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
 	struct caam_ctx *ctx = crypto_skcipher_ctx_dma(skcipher);
@@ -1365,6 +1374,13 @@ static void init_skcipher_job(struct skcipher_request *req,
 					    ivsize, encrypt);
 
 		desc_dma = dma_map_single(jrdev, desc, desc_bytes(desc), DMA_TO_DEVICE);
+		if (dma_mapping_error(jrdev, desc_dma)) {
+			dev_err(jrdev, "unable to map cipher job descriptor\n");
+			return -ENOMEM;
+		}
+
+		edesc->desc_dma = desc_dma;
+		edesc->desc_dma_bytes = desc_bytes(desc);
 
 		cnstr_desc_protected_blob_decap(edesc->hw_desc, &ctx->cdata, desc_dma);
 	} else {
@@ -1377,6 +1393,8 @@ static void init_skcipher_job(struct skcipher_request *req,
 
 		append_seq_out_ptr(desc, dst_dma, req->cryptlen + ivsize, out_options);
 	}
+
+	return 0;
 }
 
 /*
@@ -1933,7 +1951,12 @@ static inline int skcipher_crypt(struct skcipher_request *req, bool encrypt)
 		return PTR_ERR(edesc);
 
 	/* Create and submit job descriptor*/
-	init_skcipher_job(req, edesc, encrypt);
+	ret = init_skcipher_job(req, edesc, encrypt);
+	if (ret) {
+		skcipher_unmap(jrdev, edesc, req);
+		kfree(edesc);
+		return ret;
+	}
 
 	print_hex_dump_debug("skcipher jobdesc@" __stringify(__LINE__)": ",
 			     DUMP_PREFIX_ADDRESS, 16, 4, edesc->hw_desc,
-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/3] crypto: caam: Map the paes protected key once per tfm
  2026-07-26  8:15 [PATCH 1/3] crypto: caam: Fix DMA mapping leak in the cbc(paes) job path Richard Weinberger
@ 2026-07-26  8:15 ` Richard Weinberger
  2026-07-26  8:15 ` [PATCH 3/3] crypto: caam: Validate the protected key header in setkey Richard Weinberger
  2026-07-30  7:25 ` [PATCH 1/3] crypto: caam: Fix DMA mapping leak in the cbc(paes) job path Herbert Xu
  2 siblings, 0 replies; 4+ messages in thread
From: Richard Weinberger @ 2026-07-26  8:15 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-crypto, davem, herbert, gaurav.jain, pankaj.gupta,
	horia.geanta, ebiggers, upstream+linux, Richard Weinberger

paes_skcipher_setkey() maps ctx->protected_key on every call, without
checking for failure, and nothing unmaps it: a later setkey() overwrites
the address and caam_exit_common() only releases caam_init_common()'s
mapping. cbc(paes) is reachable through AF_ALG, so an unprivileged
bind()/setkey()/close() loop leaks one mapping per iteration.

The buffer is a fixed-size member of the tfm context, so map it once in
caam_cra_init() and release it in caam_exit_common(), bidirectionally as
the hardware writes the black key and reads it back through the KEY
command. A new is_paes flag marks the algorithm needing it.

Assisted-by: Claude-Code:claude-fable-5
Reported-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/crypto/caam/caamalg.c | 60 +++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 16 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index c83d28509..8826797dc 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -98,6 +98,7 @@ struct caam_alg_entry {
 	bool rfc3686;
 	bool geniv;
 	bool nodkp;
+	bool is_paes;
 };
 
 struct caam_aead_alg {
@@ -823,19 +824,6 @@ static int paes_skcipher_setkey(struct crypto_skcipher *skcipher,
 	dma_sync_single_for_device(jrdev, ctx->key_dma, keylen, DMA_TO_DEVICE);
 	ctx->cdata.key_dma = ctx->key_dma;
 
-	if (pkey_info->key_enc_algo == CAAM_ENC_ALGO_CCM)
-		ctx->protected_key_dma = dma_map_single(jrdev, ctx->protected_key,
-							ctx->cdata.plain_keylen +
-							CAAM_CCM_OVERHEAD,
-							DMA_FROM_DEVICE);
-	else
-		ctx->protected_key_dma = dma_map_single(jrdev, ctx->protected_key,
-							ctx->cdata.plain_keylen,
-							DMA_FROM_DEVICE);
-
-	ctx->cdata.protected_key_dma = ctx->protected_key_dma;
-	ctx->is_blob = true;
-
 	return 0;
 }
 
@@ -2013,6 +2001,7 @@ static struct caam_skcipher_alg driver_algs[] = {
 			.do_one_request = skcipher_do_one_req,
 		},
 		.caam.class1_alg_type = OP_ALG_ALGSEL_AES | OP_ALG_AAI_CBC,
+		.caam.is_paes = true,
 	},
 	{
 		.skcipher.base = {
@@ -3746,6 +3735,8 @@ static int caam_init_common(struct caam_ctx *ctx, struct caam_alg_entry *caam,
 	return 0;
 }
 
+static void caam_exit_common(struct caam_ctx *ctx);
+
 static int caam_cra_init(struct crypto_skcipher *tfm)
 {
 	struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
@@ -3775,10 +3766,43 @@ static int caam_cra_init(struct crypto_skcipher *tfm)
 	}
 
 	ret = caam_init_common(ctx, &caam_alg->caam, false);
-	if (ret && ctx->fallback)
-		crypto_free_skcipher(ctx->fallback);
+	if (ret) {
+		if (ctx->fallback)
+			crypto_free_skcipher(ctx->fallback);
+		return ret;
+	}
 
-	return ret;
+	if (caam_alg->caam.is_paes) {
+		/*
+		 * The hardware writes the decapsulated black key here and reads
+		 * it back through the KEY command, so map it bidirectionally,
+		 * once, for the lifetime of the tfm. Mapping it per setkey()
+		 * leaks the mapping.
+		 *
+		 * All requests of a tfm share this buffer, which is safe only
+		 * because the key cannot change while requests can be issued:
+		 * AF_ALG refuses ALG_SET_KEY once an op socket exists, so
+		 * concurrent jobs all decapsulate the same blob into the same
+		 * black key.
+		 */
+		ctx->protected_key_dma = dma_map_single(ctx->jrdev,
+							ctx->protected_key,
+							AES_MAX_KEY_SIZE,
+							DMA_BIDIRECTIONAL);
+		if (dma_mapping_error(ctx->jrdev, ctx->protected_key_dma)) {
+			dev_err(ctx->jrdev, "unable to map protected key\n");
+			ctx->protected_key_dma = 0;
+			if (ctx->fallback)
+				crypto_free_skcipher(ctx->fallback);
+			caam_exit_common(ctx);
+			return -ENOMEM;
+		}
+
+		ctx->cdata.protected_key_dma = ctx->protected_key_dma;
+		ctx->is_blob = true;
+	}
+
+	return 0;
 }
 
 static int caam_aead_init(struct crypto_aead *tfm)
@@ -3795,6 +3819,10 @@ static int caam_aead_init(struct crypto_aead *tfm)
 
 static void caam_exit_common(struct caam_ctx *ctx)
 {
+	if (ctx->protected_key_dma)
+		dma_unmap_single(ctx->jrdev, ctx->protected_key_dma,
+				 AES_MAX_KEY_SIZE, DMA_BIDIRECTIONAL);
+
 	dma_unmap_single_attrs(ctx->jrdev, ctx->sh_desc_enc_dma,
 			       offsetof(struct caam_ctx, sh_desc_enc_dma) -
 			       offsetof(struct caam_ctx, sh_desc_enc),
-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 3/3] crypto: caam: Validate the protected key header in setkey
  2026-07-26  8:15 [PATCH 1/3] crypto: caam: Fix DMA mapping leak in the cbc(paes) job path Richard Weinberger
  2026-07-26  8:15 ` [PATCH 2/3] crypto: caam: Map the paes protected key once per tfm Richard Weinberger
@ 2026-07-26  8:15 ` Richard Weinberger
  2026-07-30  7:25 ` [PATCH 1/3] crypto: caam: Fix DMA mapping leak in the cbc(paes) job path Herbert Xu
  2 siblings, 0 replies; 4+ messages in thread
From: Richard Weinberger @ 2026-07-26  8:15 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-crypto, davem, herbert, gaurav.jain, pankaj.gupta,
	horia.geanta, ebiggers, upstream+linux, Richard Weinberger

paes_skcipher_setkey() trusts struct caam_pkey_info as supplied by the
caller, which for AF_ALG means userspace. plain_key_sz is never checked
against the key material passed in, yet the blob length the hardware
reads from ctx->key is derived from it, so a short blob makes it read
data a previous key left behind, outside the range synced for the
device. Require the two to agree, and reject non-protected keys.

key_cmd_opt is OR'ed into and never reset, so KEY_EKT survives a re-key
with an ECB key. Assign it instead.

Nothing accounts for the CAAM_CCM_OVERHEAD bytes an EKT black key adds:
AES-256 CCM blobs exceed max_keysize, AES-128 ones are decapsulated
truncated, and caam_process_blob() encapsulates only part of the key.
Reject CCM until the length arithmetic is fixed end to end.

Assisted-by: Claude-Code:claude-fable-5
Reported-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/crypto/caam/caamalg.c | 52 +++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 8826797dc..d00caa9cd 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -797,6 +797,45 @@ static int paes_skcipher_setkey(struct crypto_skcipher *skcipher,
 
 	keylen = keylen - CAAM_PKEY_HEADER;
 
+	if (pkey_info->is_pkey != 1) {
+		dev_dbg(jrdev, "not a protected key\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * An EKT (CCM) black key occupies plain_key_sz + CAAM_CCM_OVERHEAD
+	 * bytes, which neither the key size range of this algorithm, nor the
+	 * blob length computed by cnstr_desc_protected_blob_decap(), nor the
+	 * KEY command length in cnstr_desc_skcipher_enc_dec() account for.
+	 * Encapsulation has the same problem: caam_process_blob() stores only
+	 * input_len bytes of an input_len + CAAM_CCM_OVERHEAD byte black key.
+	 * Reject such keys instead of submitting a job that is bound to fail
+	 * or, worse, to derive a key from a truncated blob.
+	 */
+	if (pkey_info->key_enc_algo != CAAM_ENC_ALGO_ECB) {
+		dev_dbg(jrdev, "unsupported protected key encoding %u\n",
+			pkey_info->key_enc_algo);
+		return -EOPNOTSUPP;
+	}
+
+	/* Validate key length for AES algorithms */
+	err = aes_check_keylen(pkey_info->plain_key_sz);
+	if (err) {
+		dev_dbg(jrdev, "bad key length\n");
+		return err;
+	}
+
+	/*
+	 * plain_key_sz comes straight from userspace and determines how much
+	 * data the hardware reads out of ctx->key, so it has to agree with the
+	 * amount of key material actually supplied.
+	 */
+	if (keylen != pkey_info->plain_key_sz + CAAM_BLOB_OVERHEAD) {
+		dev_dbg(jrdev, "blob length %u does not match key size %u\n",
+			keylen, pkey_info->plain_key_sz);
+		return -EINVAL;
+	}
+
 	/* Retrieve the length of key */
 	ctx->cdata.plain_keylen = pkey_info->plain_key_sz;
 
@@ -806,19 +845,8 @@ static int paes_skcipher_setkey(struct crypto_skcipher *skcipher,
 	/* Retrieve the address of the blob */
 	ctx->cdata.key_virt = pkey_info->key_buf;
 
-	/* Validate key length for AES algorithms */
-	err = aes_check_keylen(ctx->cdata.plain_keylen);
-	if (err) {
-		dev_err(jrdev, "bad key length\n");
-		return err;
-	}
-
 	/* set command option */
-	ctx->cdata.key_cmd_opt |= KEY_ENC;
-
-	/* check if the Protected-Key is CCM key */
-	if (pkey_info->key_enc_algo == CAAM_ENC_ALGO_CCM)
-		ctx->cdata.key_cmd_opt |= KEY_EKT;
+	ctx->cdata.key_cmd_opt = KEY_ENC;
 
 	memcpy(ctx->key, ctx->cdata.key_virt, keylen);
 	dma_sync_single_for_device(jrdev, ctx->key_dma, keylen, DMA_TO_DEVICE);
-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/3] crypto: caam: Fix DMA mapping leak in the cbc(paes) job path
  2026-07-26  8:15 [PATCH 1/3] crypto: caam: Fix DMA mapping leak in the cbc(paes) job path Richard Weinberger
  2026-07-26  8:15 ` [PATCH 2/3] crypto: caam: Map the paes protected key once per tfm Richard Weinberger
  2026-07-26  8:15 ` [PATCH 3/3] crypto: caam: Validate the protected key header in setkey Richard Weinberger
@ 2026-07-30  7:25 ` Herbert Xu
  2 siblings, 0 replies; 4+ messages in thread
From: Herbert Xu @ 2026-07-30  7:25 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: linux-kernel, linux-crypto, davem, gaurav.jain, pankaj.gupta,
	horia.geanta, ebiggers, upstream+linux

On Sun, Jul 26, 2026 at 10:15:02AM +0200, Richard Weinberger wrote:
> For a protected key, init_skcipher_job() maps the second job descriptor
> so that the blob decapsulation descriptor can jump to it, but keeps the
> address in a local variable. Nothing can ever unmap it, and the buffer
> it covers lives in edesc, which skcipher_crypt_done() frees while the
> mapping is still live. The mapping error is not checked either.
> 
> cbc(paes) is reachable through AF_ALG, so an unprivileged process leaks
> one mapping per request, exhausting IOVA space or swiotlb buffers.
> 
> Store the mapping in the extended descriptor, release it in
> skcipher_unmap() and propagate mapping failures to the caller.
> 
> Assisted-by: Claude-Code:claude-fable-5
> Reported-by: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  drivers/crypto/caam/caamalg.c | 31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)

Please check the Sashiko comments:

https://sashiko.dev/#/patchset/20260726081504.2182951-1-richard%40nod.at

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-30  7:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-26  8:15 [PATCH 1/3] crypto: caam: Fix DMA mapping leak in the cbc(paes) job path Richard Weinberger
2026-07-26  8:15 ` [PATCH 2/3] crypto: caam: Map the paes protected key once per tfm Richard Weinberger
2026-07-26  8:15 ` [PATCH 3/3] crypto: caam: Validate the protected key header in setkey Richard Weinberger
2026-07-30  7:25 ` [PATCH 1/3] crypto: caam: Fix DMA mapping leak in the cbc(paes) job path Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®