crypto: caam: Fix DMA mapping leak in the cbc(paes) job path
PATCH 1/3 crypto: caam: Fix DMA mapping leak in the cbc(paes) job path
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.
--- drivers/crypto/caam/caamalg.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)
drivers/crypto/caam/caamalg.c +27 -4
@@ -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,
Signature · 1 line
2.55.0
PATCH 2/3 crypto: caam: Map the paes protected key once per tfm
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.
--- drivers/crypto/caam/caamalg.c | 60 +++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 16 deletions(-)
drivers/crypto/caam/caamalg.c +44 -16
@@ -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),
Signature · 1 line
2.55.0
PATCH 3/3 crypto: caam: Validate the protected key header in setkey
In reply to Richard Weinbergerpaes_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.
--- drivers/crypto/caam/caamalg.c | 52 +++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 12 deletions(-)
drivers/crypto/caam/caamalg.c +40 -12
@@ -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 );
Signature · 1 line
2.55.0
Herbert Xu
maintainerQuoted 12 lines · 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, whichskcipher_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.
Quoted 6 lines
Assisted-by Claude-Code:claude-fable-5Reported-by Eric Biggers <[email protected]>Signed-off-by Richard Weinberger <[email protected]>--- 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,
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