crypto: caam - fix MDHA key derivation for certain user key lengths

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ă <horia.geanta@nxp.com>
Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 21e30ded365a8471ba8222cef5050ca8fda99247..947ba8ef487a584c048adfceb170ea841dab91a6 100644 (file)
--- 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 e63b2f71969578345fc56eb807f4c179f0fb2d2d..59b59f5e9550f6399ce6ec585226a62dd0649d44 100644 (file)
--- 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 63a86b6b8b96b0e7afd04083ab0b80293af3b42c..bd01bcd799e8605688f5bdf127c5aee713dd051e 100644 (file)
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -199,6 +199,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;
 
@@ -210,16 +222,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);
 
@@ -248,16 +250,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);
 
@@ -2999,6 +2991,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
@@ -3006,6 +2999,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;
@@ -3306,6 +3300,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;
@@ -4536,11 +4543,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;
        }
 
@@ -4566,6 +4589,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 d19373e6ed4a4bb5b75f8a7308b0ae8168a37dd4..ed1931f97df14ac57aae038e14005e3a5d1971ab 100644 (file)
--- 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 815417411a1801c99ba96e50b417648924d898f9..536f360bf1317a841707201f184d6c4631331a96 100644 (file)
--- 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 c6f8375ae215ff27e6c4f851a7ecb513ff9e91ce..5a851ddc48fbed2cd706956eb5a90fe2f2803b2c 100644 (file)
--- 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;