The series solves:
- the failures found with fuzz testing;
- resources clean-up on caampkc/caamrng exit path.
The first 10 patches solve the issues found with
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS enabled.
They modify the drivers to provide a valid error (and not the hardware
error ID) to the user, via completion callbacks.
They check key length, assoclen, authsize and input size to solve the
fuzz tests that expect -EINVAL to be returned when these values are
not valid.
The next 4 patches check the algorithm registration for caampkc
module and unregister it only if the registration was successful.
Also, on caampkc/caamrng, the exit point function is executed only if the
registration was successful to avoid double freeing of resources in case
the initialization function failed.
This patch depends on series:
https://patchwork.kernel.org/project/linux-crypto/list/?series=150651
Changes since v2:
- use helper functions from crypto API, to validate the inputs;
- update rfc4106 shared descriptor with the erratum workaround;
- fix MDHA key derivation for CAAM with era < 6;
- remove check for keylen < 4, since is included in check_aes_keylen.
Horia Geantă (5):
crypto: caam/qi - fix error handling in ERN handler
crypto: caam - fix return code in completion callbacks
crypto: caam - update IV only when crypto operation succeeds
crypto: caam - keep both virtual and dma key addresses
crypto: caam - fix MDHA key derivation for certain user key lengths
Iuliana Prodan (9):
crypto: caam - check key length
crypto: caam - check authsize
crypto: caam - check assoclen
crypto: caam - check zero-length input
crypto: caam - update rfc4106 sh desc to support zero length input
crypto: caam - free resources in case caam_rng registration failed
crypto: caam - execute module exit point only if necessary
crypto: caam - unregister algorithm only if the registration succeeded
crypto: caam - change return value in case CAAM has no MDHA
drivers/crypto/caam/Kconfig | 2 +
drivers/crypto/caam/caamalg.c | 227 +++++++++++++++----------
drivers/crypto/caam/caamalg_desc.c | 45 +++--
drivers/crypto/caam/caamalg_desc.h | 2 +-
drivers/crypto/caam/caamalg_qi.c | 223 +++++++++++++++----------
drivers/crypto/caam/caamalg_qi2.c | 320 +++++++++++++++++++++++-------------
drivers/crypto/caam/caamhash.c | 114 ++++++++-----
drivers/crypto/caam/caamhash_desc.c | 5 +-
drivers/crypto/caam/caamhash_desc.h | 2 +-
drivers/crypto/caam/caampkc.c | 80 ++++++---
drivers/crypto/caam/caamrng.c | 17 +-
drivers/crypto/caam/desc_constr.h | 34 ++--
drivers/crypto/caam/error.c | 61 ++++---
drivers/crypto/caam/error.h | 2 +-
drivers/crypto/caam/key_gen.c | 14 +-
drivers/crypto/caam/qi.c | 10 +-
drivers/crypto/caam/regs.h | 1 +
17 files changed, 745 insertions(+), 414 deletions(-)
--
2.1.0
From: Horia Geantă <[email protected]>
skcipher encryption might fail and in some cases, like (invalid) input
length smaller then block size, updating the IV would lead to panic
due to copying from a negative offset (req->cryptlen - ivsize).
Signed-off-by: Horia Geantă <[email protected]>
Signed-off-by: Iuliana Prodan <[email protected]>
---
drivers/crypto/caam/caamalg.c | 5 ++---
drivers/crypto/caam/caamalg_qi.c | 4 +++-
drivers/crypto/caam/caamalg_qi2.c | 8 ++++++--
3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 06b4f2d..28d55a0 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -990,10 +990,9 @@ static void skcipher_encrypt_done(struct device *jrdev, u32 *desc, u32 err,
* ciphertext block (CBC mode) or last counter (CTR mode).
* This is used e.g. by the CTS mode.
*/
- if (ivsize) {
+ if (ivsize && !ecode) {
memcpy(req->iv, (u8 *)edesc->sec4_sg + edesc->sec4_sg_bytes,
ivsize);
-
print_hex_dump_debug("dstiv @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->iv,
edesc->src_nents > 1 ? 100 : ivsize, 1);
@@ -1030,7 +1029,7 @@ static void skcipher_decrypt_done(struct device *jrdev, u32 *desc, u32 err,
* ciphertext block (CBC mode) or last counter (CTR mode).
* This is used e.g. by the CTS mode.
*/
- if (ivsize) {
+ if (ivsize && !ecode) {
memcpy(req->iv, (u8 *)edesc->sec4_sg + edesc->sec4_sg_bytes,
ivsize);
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index ab263b1..66531d6 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -1201,7 +1201,9 @@ static void skcipher_done(struct caam_drv_req *drv_req, u32 status)
* ciphertext block (CBC mode) or last counter (CTR mode).
* This is used e.g. by the CTS mode.
*/
- memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes, ivsize);
+ if (!ecode)
+ memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes,
+ ivsize);
qi_cache_free(edesc);
skcipher_request_complete(req, ecode);
diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c
index 2681581..bc370af 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -1358,7 +1358,9 @@ static void skcipher_encrypt_done(void *cbk_ctx, u32 status)
* ciphertext block (CBC mode) or last counter (CTR mode).
* This is used e.g. by the CTS mode.
*/
- memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes, ivsize);
+ if (!ecode)
+ memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes,
+ ivsize);
qi_cache_free(edesc);
skcipher_request_complete(req, ecode);
@@ -1394,7 +1396,9 @@ static void skcipher_decrypt_done(void *cbk_ctx, u32 status)
* ciphertext block (CBC mode) or last counter (CTR mode).
* This is used e.g. by the CTS mode.
*/
- memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes, ivsize);
+ if (!ecode)
+ memcpy(req->iv, (u8 *)&edesc->sgt[0] + edesc->qm_sg_bytes,
+ ivsize);
qi_cache_free(edesc);
skcipher_request_complete(req, ecode);
--
2.1.0
From: Horia Geantă <[email protected]>
Fuzz testing uncovered an issue when |user key| > |derived key|.
Derived key generation has to be fixed in two cases:
1. Era >= 6 (DKP is available)
DKP cannot be used with immediate input key if |user key| > |derived key|,
since the resulting descriptor (after DKP execution) would be invalid -
having a few bytes from user key left in descriptor buffer
as incorrect opcodes.
Fix DKP usage both in standalone hmac and in authenc algorithms.
For authenc the logic is simplified, by always storing both virtual
and dma key addresses.
2. Era < 6
The same case (|user key| > |derived key|) fails when DKP
is not available.
Make sure gen_split_key() dma maps max(|user key|, |derived key|),
since this is an in-place (bidirectional) operation.
Signed-off-by: Horia Geantă <[email protected]>
Changes since v2:
- fix MDHA key derivation for CAAM with era < 6.
---
drivers/crypto/caam/caamalg.c | 42 +++++++-----------------
drivers/crypto/caam/caamalg_qi.c | 42 +++++++-----------------
drivers/crypto/caam/caamalg_qi2.c | 67 +++++++++++++++++++++++++++------------
drivers/crypto/caam/caamhash.c | 53 ++++++++++++++++++++++---------
drivers/crypto/caam/desc_constr.h | 24 ++++++++++----
drivers/crypto/caam/key_gen.c | 9 +++---
6 files changed, 132 insertions(+), 105 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index b14a13b..3cc9d5c 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -205,6 +205,18 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ctx->cdata.keylen - CTR_RFC3686_NONCE_SIZE);
}
+ /*
+ * In case |user key| > |derived key|, using DKP<imm,imm>
+ * would result in invalid opcodes (last bytes of user key) in
+ * the resulting descriptor. Use DKP<ptr,imm> instead => both
+ * virtual and dma key addresses are needed.
+ */
+ ctx->adata.key_virt = ctx->key;
+ ctx->adata.key_dma = ctx->key_dma;
+
+ ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
+ ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
+
data_len[0] = ctx->adata.keylen_pad;
data_len[1] = ctx->cdata.keylen;
@@ -221,16 +233,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;
- if (inl_mask & 1)
- ctx->adata.key_virt = ctx->key;
- else
- ctx->adata.key_dma = ctx->key_dma;
-
- if (inl_mask & 2)
- ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
- else
- ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
-
ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);
@@ -253,16 +255,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;
- if (inl_mask & 1)
- ctx->adata.key_virt = ctx->key;
- else
- ctx->adata.key_dma = ctx->key_dma;
-
- if (inl_mask & 2)
- ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
- else
- ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
-
ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);
@@ -287,16 +279,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;
- if (inl_mask & 1)
- ctx->adata.key_virt = ctx->key;
- else
- ctx->adata.key_dma = ctx->key_dma;
-
- if (inl_mask & 2)
- ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
- else
- ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
-
ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index 452467c..6fe6713 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -105,6 +105,18 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ctx->cdata.keylen - CTR_RFC3686_NONCE_SIZE);
}
+ /*
+ * In case |user key| > |derived key|, using DKP<imm,imm> would result
+ * in invalid opcodes (last bytes of user key) in the resulting
+ * descriptor. Use DKP<ptr,imm> instead => both virtual and dma key
+ * addresses are needed.
+ */
+ ctx->adata.key_virt = ctx->key;
+ ctx->adata.key_dma = ctx->key_dma;
+
+ ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
+ ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
+
data_len[0] = ctx->adata.keylen_pad;
data_len[1] = ctx->cdata.keylen;
@@ -118,16 +130,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;
- if (inl_mask & 1)
- ctx->adata.key_virt = ctx->key;
- else
- ctx->adata.key_dma = ctx->key_dma;
-
- if (inl_mask & 2)
- ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
- else
- ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
-
ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);
@@ -143,16 +145,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;
- if (inl_mask & 1)
- ctx->adata.key_virt = ctx->key;
- else
- ctx->adata.key_dma = ctx->key_dma;
-
- if (inl_mask & 2)
- ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
- else
- ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
-
ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);
@@ -171,16 +163,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;
- if (inl_mask & 1)
- ctx->adata.key_virt = ctx->key;
- else
- ctx->adata.key_dma = ctx->key_dma;
-
- if (inl_mask & 2)
- ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
- else
- ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
-
ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);
diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c
index c1eca99..66228e7 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -198,6 +198,18 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ctx->cdata.keylen - CTR_RFC3686_NONCE_SIZE);
}
+ /*
+ * In case |user key| > |derived key|, using DKP<imm,imm> would result
+ * in invalid opcodes (last bytes of user key) in the resulting
+ * descriptor. Use DKP<ptr,imm> instead => both virtual and dma key
+ * addresses are needed.
+ */
+ ctx->adata.key_virt = ctx->key;
+ ctx->adata.key_dma = ctx->key_dma;
+
+ ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
+ ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
+
data_len[0] = ctx->adata.keylen_pad;
data_len[1] = ctx->cdata.keylen;
@@ -209,16 +221,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;
- if (inl_mask & 1)
- ctx->adata.key_virt = ctx->key;
- else
- ctx->adata.key_dma = ctx->key_dma;
-
- if (inl_mask & 2)
- ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
- else
- ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
-
ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);
@@ -247,16 +249,6 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
ARRAY_SIZE(data_len)) < 0)
return -EINVAL;
- if (inl_mask & 1)
- ctx->adata.key_virt = ctx->key;
- else
- ctx->adata.key_dma = ctx->key_dma;
-
- if (inl_mask & 2)
- ctx->cdata.key_virt = ctx->key + ctx->adata.keylen_pad;
- else
- ctx->cdata.key_dma = ctx->key_dma + ctx->adata.keylen_pad;
-
ctx->adata.key_inline = !!(inl_mask & 1);
ctx->cdata.key_inline = !!(inl_mask & 2);
@@ -2998,6 +2990,7 @@ enum hash_optype {
/**
* caam_hash_ctx - ahash per-session context
* @flc: Flow Contexts array
+ * @key: authentication key
* @flc_dma: I/O virtual addresses of the Flow Contexts
* @dev: dpseci device
* @ctx_len: size of Context Register
@@ -3005,6 +2998,7 @@ enum hash_optype {
*/
struct caam_hash_ctx {
struct caam_flc flc[HASH_NUM_OP];
+ u8 key[CAAM_MAX_HASH_BLOCK_SIZE] ____cacheline_aligned;
dma_addr_t flc_dma[HASH_NUM_OP];
struct device *dev;
int ctx_len;
@@ -3305,6 +3299,19 @@ static int ahash_setkey(struct crypto_ahash *ahash, const u8 *key,
ctx->adata.key_virt = key;
ctx->adata.key_inline = true;
+ /*
+ * In case |user key| > |derived key|, using DKP<imm,imm> would result
+ * in invalid opcodes (last bytes of user key) in the resulting
+ * descriptor. Use DKP<ptr,imm> instead => both virtual and dma key
+ * addresses are needed.
+ */
+ if (keylen > ctx->adata.keylen_pad) {
+ memcpy(ctx->key, key, keylen);
+ dma_sync_single_for_device(ctx->dev, ctx->adata.key_dma,
+ ctx->adata.keylen_pad,
+ DMA_TO_DEVICE);
+ }
+
ret = ahash_set_sh_desc(ahash);
kfree(hashed_key);
return ret;
@@ -4535,11 +4542,27 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm)
ctx->dev = caam_hash->dev;
+ if (alg->setkey) {
+ ctx->adata.key_dma = dma_map_single_attrs(ctx->dev, ctx->key,
+ ARRAY_SIZE(ctx->key),
+ DMA_TO_DEVICE,
+ DMA_ATTR_SKIP_CPU_SYNC);
+ if (dma_mapping_error(ctx->dev, ctx->adata.key_dma)) {
+ dev_err(ctx->dev, "unable to map key\n");
+ return -ENOMEM;
+ }
+ }
+
dma_addr = dma_map_single_attrs(ctx->dev, ctx->flc, sizeof(ctx->flc),
DMA_BIDIRECTIONAL,
DMA_ATTR_SKIP_CPU_SYNC);
if (dma_mapping_error(ctx->dev, dma_addr)) {
dev_err(ctx->dev, "unable to map shared descriptors\n");
+ if (ctx->adata.key_dma)
+ dma_unmap_single_attrs(ctx->dev, ctx->adata.key_dma,
+ ARRAY_SIZE(ctx->key),
+ DMA_TO_DEVICE,
+ DMA_ATTR_SKIP_CPU_SYNC);
return -ENOMEM;
}
@@ -4565,6 +4588,10 @@ static void caam_hash_cra_exit(struct crypto_tfm *tfm)
dma_unmap_single_attrs(ctx->dev, ctx->flc_dma[0], sizeof(ctx->flc),
DMA_BIDIRECTIONAL, DMA_ATTR_SKIP_CPU_SYNC);
+ if (ctx->adata.key_dma)
+ dma_unmap_single_attrs(ctx->dev, ctx->adata.key_dma,
+ ARRAY_SIZE(ctx->key), DMA_TO_DEVICE,
+ DMA_ATTR_SKIP_CPU_SYNC);
}
static struct caam_hash_alg *caam_hash_alloc(struct device *dev,
diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 49ab6ca..7147190 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -96,6 +96,7 @@ struct caam_hash_ctx {
dma_addr_t sh_desc_fin_dma;
dma_addr_t sh_desc_digest_dma;
enum dma_data_direction dir;
+ enum dma_data_direction key_dir;
struct device *jrdev;
int ctx_len;
struct alginfo adata;
@@ -476,6 +477,18 @@ static int ahash_setkey(struct crypto_ahash *ahash,
goto bad_free_key;
memcpy(ctx->key, key, keylen);
+
+ /*
+ * In case |user key| > |derived key|, using DKP<imm,imm>
+ * would result in invalid opcodes (last bytes of user key) in
+ * the resulting descriptor. Use DKP<ptr,imm> instead => both
+ * virtual and dma key addresses are needed.
+ */
+ if (keylen > ctx->adata.keylen_pad)
+ dma_sync_single_for_device(ctx->jrdev,
+ ctx->adata.key_dma,
+ ctx->adata.keylen_pad,
+ DMA_TO_DEVICE);
} else {
ret = gen_split_key(ctx->jrdev, ctx->key, &ctx->adata, key,
keylen, CAAM_MAX_HASH_KEY_SIZE);
@@ -1825,40 +1838,50 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm)
if (is_xcbc_aes(caam_hash->alg_type)) {
ctx->dir = DMA_TO_DEVICE;
+ ctx->key_dir = DMA_BIDIRECTIONAL;
ctx->adata.algtype = OP_TYPE_CLASS1_ALG | caam_hash->alg_type;
ctx->ctx_len = 48;
-
- ctx->adata.key_dma = dma_map_single_attrs(ctx->jrdev, ctx->key,
- ARRAY_SIZE(ctx->key),
- DMA_BIDIRECTIONAL,
- DMA_ATTR_SKIP_CPU_SYNC);
- if (dma_mapping_error(ctx->jrdev, ctx->adata.key_dma)) {
- dev_err(ctx->jrdev, "unable to map key\n");
- caam_jr_free(ctx->jrdev);
- return -ENOMEM;
- }
} else if (is_cmac_aes(caam_hash->alg_type)) {
ctx->dir = DMA_TO_DEVICE;
+ ctx->key_dir = DMA_NONE;
ctx->adata.algtype = OP_TYPE_CLASS1_ALG | caam_hash->alg_type;
ctx->ctx_len = 32;
} else {
- ctx->dir = priv->era >= 6 ? DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
+ if (priv->era >= 6) {
+ ctx->dir = DMA_BIDIRECTIONAL;
+ ctx->key_dir = alg->setkey ? DMA_TO_DEVICE : DMA_NONE;
+ } else {
+ ctx->dir = DMA_TO_DEVICE;
+ ctx->key_dir = DMA_NONE;
+ }
ctx->adata.algtype = OP_TYPE_CLASS2_ALG | caam_hash->alg_type;
ctx->ctx_len = runninglen[(ctx->adata.algtype &
OP_ALG_ALGSEL_SUBMASK) >>
OP_ALG_ALGSEL_SHIFT];
}
+ if (ctx->key_dir != DMA_NONE) {
+ ctx->adata.key_dma = dma_map_single_attrs(ctx->jrdev, ctx->key,
+ ARRAY_SIZE(ctx->key),
+ ctx->key_dir,
+ DMA_ATTR_SKIP_CPU_SYNC);
+ if (dma_mapping_error(ctx->jrdev, ctx->adata.key_dma)) {
+ dev_err(ctx->jrdev, "unable to map key\n");
+ caam_jr_free(ctx->jrdev);
+ return -ENOMEM;
+ }
+ }
+
dma_addr = dma_map_single_attrs(ctx->jrdev, ctx->sh_desc_update,
offsetof(struct caam_hash_ctx, key),
ctx->dir, DMA_ATTR_SKIP_CPU_SYNC);
if (dma_mapping_error(ctx->jrdev, dma_addr)) {
dev_err(ctx->jrdev, "unable to map shared descriptors\n");
- if (is_xcbc_aes(caam_hash->alg_type))
+ if (ctx->key_dir != DMA_NONE)
dma_unmap_single_attrs(ctx->jrdev, ctx->adata.key_dma,
ARRAY_SIZE(ctx->key),
- DMA_BIDIRECTIONAL,
+ ctx->key_dir,
DMA_ATTR_SKIP_CPU_SYNC);
caam_jr_free(ctx->jrdev);
@@ -1891,9 +1914,9 @@ static void caam_hash_cra_exit(struct crypto_tfm *tfm)
dma_unmap_single_attrs(ctx->jrdev, ctx->sh_desc_update_dma,
offsetof(struct caam_hash_ctx, key),
ctx->dir, DMA_ATTR_SKIP_CPU_SYNC);
- if (is_xcbc_aes(ctx->adata.algtype))
+ if (ctx->key_dir != DMA_NONE)
dma_unmap_single_attrs(ctx->jrdev, ctx->adata.key_dma,
- ARRAY_SIZE(ctx->key), DMA_BIDIRECTIONAL,
+ ARRAY_SIZE(ctx->key), ctx->key_dir,
DMA_ATTR_SKIP_CPU_SYNC);
caam_jr_free(ctx->jrdev);
}
diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h
index 8154174..536f360 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -533,14 +533,26 @@ static inline void append_proto_dkp(u32 * const desc, struct alginfo *adata)
if (adata->key_inline) {
int words;
- append_operation(desc, OP_TYPE_UNI_PROTOCOL | protid |
- OP_PCL_DKP_SRC_IMM | OP_PCL_DKP_DST_IMM |
- adata->keylen);
- append_data(desc, adata->key_virt, adata->keylen);
+ if (adata->keylen > adata->keylen_pad) {
+ append_operation(desc, OP_TYPE_UNI_PROTOCOL | protid |
+ OP_PCL_DKP_SRC_PTR |
+ OP_PCL_DKP_DST_IMM | adata->keylen);
+ append_ptr(desc, adata->key_dma);
+
+ words = (ALIGN(adata->keylen_pad, CAAM_CMD_SZ) -
+ CAAM_PTR_SZ) / CAAM_CMD_SZ;
+ } else {
+ append_operation(desc, OP_TYPE_UNI_PROTOCOL | protid |
+ OP_PCL_DKP_SRC_IMM |
+ OP_PCL_DKP_DST_IMM | adata->keylen);
+ append_data(desc, adata->key_virt, adata->keylen);
+
+ words = (ALIGN(adata->keylen_pad, CAAM_CMD_SZ) -
+ ALIGN(adata->keylen, CAAM_CMD_SZ)) /
+ CAAM_CMD_SZ;
+ }
/* Reserve space in descriptor buffer for the derived key */
- words = (ALIGN(adata->keylen_pad, CAAM_CMD_SZ) -
- ALIGN(adata->keylen, CAAM_CMD_SZ)) / CAAM_CMD_SZ;
if (words)
(*desc) = cpu_to_caam32(caam32_to_cpu(*desc) + words);
} else {
diff --git a/drivers/crypto/caam/key_gen.c b/drivers/crypto/caam/key_gen.c
index c6f8375..5a851dd 100644
--- a/drivers/crypto/caam/key_gen.c
+++ b/drivers/crypto/caam/key_gen.c
@@ -48,18 +48,20 @@ int gen_split_key(struct device *jrdev, u8 *key_out,
u32 *desc;
struct split_key_result result;
dma_addr_t dma_addr;
+ unsigned int local_max;
int ret = -ENOMEM;
adata->keylen = split_key_len(adata->algtype & OP_ALG_ALGSEL_MASK);
adata->keylen_pad = split_key_pad_len(adata->algtype &
OP_ALG_ALGSEL_MASK);
+ local_max = max(keylen, adata->keylen_pad);
dev_dbg(jrdev, "split keylen %d split keylen padded %d\n",
adata->keylen, adata->keylen_pad);
print_hex_dump_debug("ctx.key@" __stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key_in, keylen, 1);
- if (adata->keylen_pad > max_keylen)
+ if (local_max > max_keylen)
return -EINVAL;
desc = kmalloc(CAAM_CMD_SZ * 6 + CAAM_PTR_SZ * 2, GFP_KERNEL | GFP_DMA);
@@ -70,8 +72,7 @@ int gen_split_key(struct device *jrdev, u8 *key_out,
memcpy(key_out, key_in, keylen);
- dma_addr = dma_map_single(jrdev, key_out, adata->keylen_pad,
- DMA_BIDIRECTIONAL);
+ dma_addr = dma_map_single(jrdev, key_out, local_max, DMA_BIDIRECTIONAL);
if (dma_mapping_error(jrdev, dma_addr)) {
dev_err(jrdev, "unable to map key memory\n");
goto out_free;
@@ -117,7 +118,7 @@ int gen_split_key(struct device *jrdev, u8 *key_out,
adata->keylen_pad, 1);
}
- dma_unmap_single(jrdev, dma_addr, adata->keylen_pad, DMA_BIDIRECTIONAL);
+ dma_unmap_single(jrdev, dma_addr, local_max, DMA_BIDIRECTIONAL);
out_free:
kfree(desc);
return ret;
--
2.1.0
Check zero-length input, for skcipher algorithm, to solve the extra
tests. This is a valid operation, therefore the API will return no error.
Signed-off-by: Iuliana Prodan <[email protected]>
Reviewed-by: Horia Geanta <[email protected]>
---
drivers/crypto/caam/caamalg.c | 6 ++++++
drivers/crypto/caam/caamalg_qi.c | 3 +++
drivers/crypto/caam/caamalg_qi2.c | 5 +++++
3 files changed, 14 insertions(+)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index a5fcc31..b14a13b 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -1884,6 +1884,9 @@ static int skcipher_encrypt(struct skcipher_request *req)
u32 *desc;
int ret = 0;
+ if (!req->cryptlen)
+ return 0;
+
/* allocate extended descriptor */
edesc = skcipher_edesc_alloc(req, DESC_JOB_IO_LEN * CAAM_CMD_SZ);
if (IS_ERR(edesc))
@@ -1918,6 +1921,9 @@ static int skcipher_decrypt(struct skcipher_request *req)
u32 *desc;
int ret = 0;
+ if (!req->cryptlen)
+ return 0;
+
/* allocate extended descriptor */
edesc = skcipher_edesc_alloc(req, DESC_JOB_IO_LEN * CAAM_CMD_SZ);
if (IS_ERR(edesc))
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index b8de0a8..452467c 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -1443,6 +1443,9 @@ static inline int skcipher_crypt(struct skcipher_request *req, bool encrypt)
struct caam_ctx *ctx = crypto_skcipher_ctx(skcipher);
int ret;
+ if (!req->cryptlen)
+ return 0;
+
if (unlikely(caam_congested))
return -EAGAIN;
diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c
index fb6c757..c1eca99 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -1498,6 +1498,9 @@ static int skcipher_encrypt(struct skcipher_request *req)
struct caam_request *caam_req = skcipher_request_ctx(req);
int ret;
+ if (!req->cryptlen)
+ return 0;
+
/* allocate extended descriptor */
edesc = skcipher_edesc_alloc(req);
if (IS_ERR(edesc))
@@ -1526,6 +1529,8 @@ static int skcipher_decrypt(struct skcipher_request *req)
struct caam_request *caam_req = skcipher_request_ctx(req);
int ret;
+ if (!req->cryptlen)
+ return 0;
/* allocate extended descriptor */
edesc = skcipher_edesc_alloc(req);
if (IS_ERR(edesc))
--
2.1.0
Check authsize to solve the extra tests that expect -EINVAL to be
returned when the authentication tag size is not valid.
Validated authsize for GCM, RFC4106 and RFC4543.
Signed-off-by: Iuliana Prodan <[email protected]>
---
drivers/crypto/caam/caamalg.c | 13 +++++++++++++
drivers/crypto/caam/caamalg_qi.c | 13 +++++++++++++
drivers/crypto/caam/caamalg_qi2.c | 13 +++++++++++++
3 files changed, 39 insertions(+)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index d379ea6..dcc1b89 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -376,6 +376,11 @@ static int gcm_set_sh_desc(struct crypto_aead *aead)
static int gcm_setauthsize(struct crypto_aead *authenc, unsigned int authsize)
{
struct caam_ctx *ctx = crypto_aead_ctx(authenc);
+ int err;
+
+ err = check_gcm_authsize(authsize);
+ if (err)
+ return err;
ctx->authsize = authsize;
gcm_set_sh_desc(authenc);
@@ -439,6 +444,11 @@ static int rfc4106_setauthsize(struct crypto_aead *authenc,
unsigned int authsize)
{
struct caam_ctx *ctx = crypto_aead_ctx(authenc);
+ int err;
+
+ err = check_rfc4106_authsize(authsize);
+ if (err)
+ return err;
ctx->authsize = authsize;
rfc4106_set_sh_desc(authenc);
@@ -503,6 +513,9 @@ static int rfc4543_setauthsize(struct crypto_aead *authenc,
{
struct caam_ctx *ctx = crypto_aead_ctx(authenc);
+ if (authsize != 16)
+ return -EINVAL;
+
ctx->authsize = authsize;
rfc4543_set_sh_desc(authenc);
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index 6d8cdba..8bc7564 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -371,6 +371,11 @@ static int gcm_set_sh_desc(struct crypto_aead *aead)
static int gcm_setauthsize(struct crypto_aead *authenc, unsigned int authsize)
{
struct caam_ctx *ctx = crypto_aead_ctx(authenc);
+ int err;
+
+ err = check_gcm_authsize(authsize);
+ if (err)
+ return err;
ctx->authsize = authsize;
gcm_set_sh_desc(authenc);
@@ -472,6 +477,11 @@ static int rfc4106_setauthsize(struct crypto_aead *authenc,
unsigned int authsize)
{
struct caam_ctx *ctx = crypto_aead_ctx(authenc);
+ int err;
+
+ err = check_rfc4106_authsize(authsize);
+ if (err)
+ return err;
ctx->authsize = authsize;
rfc4106_set_sh_desc(authenc);
@@ -578,6 +588,9 @@ static int rfc4543_setauthsize(struct crypto_aead *authenc,
{
struct caam_ctx *ctx = crypto_aead_ctx(authenc);
+ if (authsize != 16)
+ return -EINVAL;
+
ctx->authsize = authsize;
rfc4543_set_sh_desc(authenc);
diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c
index a13d05e..faea744 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -719,6 +719,11 @@ static int gcm_set_sh_desc(struct crypto_aead *aead)
static int gcm_setauthsize(struct crypto_aead *authenc, unsigned int authsize)
{
struct caam_ctx *ctx = crypto_aead_ctx(authenc);
+ int err;
+
+ err = check_gcm_authsize(authsize);
+ if (err)
+ return err;
ctx->authsize = authsize;
gcm_set_sh_desc(authenc);
@@ -811,6 +816,11 @@ static int rfc4106_setauthsize(struct crypto_aead *authenc,
unsigned int authsize)
{
struct caam_ctx *ctx = crypto_aead_ctx(authenc);
+ int err;
+
+ err = check_rfc4106_authsize(authsize);
+ if (err)
+ return err;
ctx->authsize = authsize;
rfc4106_set_sh_desc(authenc);
@@ -910,6 +920,9 @@ static int rfc4543_setauthsize(struct crypto_aead *authenc,
{
struct caam_ctx *ctx = crypto_aead_ctx(authenc);
+ if (authsize != 16)
+ return -EINVAL;
+
ctx->authsize = authsize;
rfc4543_set_sh_desc(authenc);
--
2.1.0
Check assoclen to solve the extra tests that expect -EINVAL to be
returned when the associated data size is not valid.
Validated assoclen for RFC4106 and RFC4543 which expects an assoclen
of 16 or 20.
Based on seqiv, IPsec ESP and RFC4543/RFC4106 the assoclen is sizeof IP
Header (spi, seq_no, extended seq_no) and IV len. This can be 16 or 20
bytes.
Signed-off-by: Iuliana Prodan <[email protected]>
---
drivers/crypto/caam/caamalg.c | 10 ++--------
drivers/crypto/caam/caamalg_qi.c | 10 ++--------
drivers/crypto/caam/caamalg_qi2.c | 10 ++--------
3 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index dcc1b89..a5fcc31 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -1598,10 +1598,7 @@ static int chachapoly_decrypt(struct aead_request *req)
static int ipsec_gcm_encrypt(struct aead_request *req)
{
- if (req->assoclen < 8)
- return -EINVAL;
-
- return gcm_encrypt(req);
+ return check_ipsec_assoclen(req->assoclen) ? : gcm_encrypt(req);
}
static int aead_encrypt(struct aead_request *req)
@@ -1675,10 +1672,7 @@ static int gcm_decrypt(struct aead_request *req)
static int ipsec_gcm_decrypt(struct aead_request *req)
{
- if (req->assoclen < 8)
- return -EINVAL;
-
- return gcm_decrypt(req);
+ return check_ipsec_assoclen(req->assoclen) ? : gcm_decrypt(req);
}
static int aead_decrypt(struct aead_request *req)
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index 8bc7564..b8de0a8 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -1237,18 +1237,12 @@ static int aead_decrypt(struct aead_request *req)
static int ipsec_gcm_encrypt(struct aead_request *req)
{
- if (req->assoclen < 8)
- return -EINVAL;
-
- return aead_crypt(req, true);
+ return check_ipsec_assoclen(req->assoclen) ? : aead_crypt(req, true);
}
static int ipsec_gcm_decrypt(struct aead_request *req)
{
- if (req->assoclen < 8)
- return -EINVAL;
-
- return aead_crypt(req, false);
+ return check_ipsec_assoclen(req->assoclen) ? : aead_crypt(req, false);
}
static void skcipher_done(struct caam_drv_req *drv_req, u32 status)
diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c
index faea744..fb6c757 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -1406,18 +1406,12 @@ static int aead_decrypt(struct aead_request *req)
static int ipsec_gcm_encrypt(struct aead_request *req)
{
- if (req->assoclen < 8)
- return -EINVAL;
-
- return aead_encrypt(req);
+ return check_ipsec_assoclen(req->assoclen) ? : aead_encrypt(req);
}
static int ipsec_gcm_decrypt(struct aead_request *req)
{
- if (req->assoclen < 8)
- return -EINVAL;
-
- return aead_decrypt(req);
+ return check_ipsec_assoclen(req->assoclen) ? : aead_decrypt(req);
}
static void skcipher_encrypt_done(void *cbk_ctx, u32 status)
--
2.1.0
On 7/25/2019 4:58 PM, Iuliana Prodan wrote:
> From: Horia Geant? <[email protected]>
>
> skcipher encryption might fail and in some cases, like (invalid) input
> length smaller then block size, updating the IV would lead to panic
> due to copying from a negative offset (req->cryptlen - ivsize).
>
In v1 I mentioned the commit message needs to change:
https://lore.kernel.org/linux-crypto/VI1PR0402MB34855E675A58E1221ACE7B9498C80@VI1PR0402MB3485.eurprd04.prod.outlook.com/
which happened in v2, however somehow v3 is identical to v1.
Horia
On 7/25/2019 4:58 PM, Iuliana Prodan wrote:
> From: Horia Geant? <[email protected]>
>
> Fuzz testing uncovered an issue when |user key| > |derived key|.
> Derived key generation has to be fixed in two cases:
>
> 1. Era >= 6 (DKP is available)
> DKP cannot be used with immediate input key if |user key| > |derived key|,
> since the resulting descriptor (after DKP execution) would be invalid -
> having a few bytes from user key left in descriptor buffer
> as incorrect opcodes.
>
> Fix DKP usage both in standalone hmac and in authenc algorithms.
> For authenc the logic is simplified, by always storing both virtual
> and dma key addresses.
>
> 2. Era < 6
> The same case (|user key| > |derived key|) fails when DKP
> is not available.
> Make sure gen_split_key() dma maps max(|user key|, |derived key|),
> since this is an in-place (bidirectional) operation.
>
> Signed-off-by: Horia Geant? <[email protected]>
>
> Changes since v2:
> - fix MDHA key derivation for CAAM with era < 6.
> ---
The change log shouldn't be included in the commit message.
Horia
On 7/25/2019 4:58 PM, Iuliana Prodan wrote:
> Check authsize to solve the extra tests that expect -EINVAL to be
> returned when the authentication tag size is not valid.
>
> Validated authsize for GCM, RFC4106 and RFC4543.
>
> Signed-off-by: Iuliana Prodan <[email protected]>
Reviewed-by: Horia Geant? <[email protected]>
Thanks,
Horia
On 7/25/2019 4:58 PM, Iuliana Prodan wrote:
> Check assoclen to solve the extra tests that expect -EINVAL to be
> returned when the associated data size is not valid.
>
> Validated assoclen for RFC4106 and RFC4543 which expects an assoclen
> of 16 or 20.
> Based on seqiv, IPsec ESP and RFC4543/RFC4106 the assoclen is sizeof IP
> Header (spi, seq_no, extended seq_no) and IV len. This can be 16 or 20
> bytes.
>
> Signed-off-by: Iuliana Prodan <[email protected]>
Reviewed-by: Horia Geant? <[email protected]>
Thanks,
Horia