2019-07-18 14:45:56

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 00/14] crypto: caam - fixes for kernel v5.3

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.

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 DKP for certain 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/Makefile | 2 +-
drivers/crypto/caam/caamalg.c | 226 ++++++++++++++++----------
drivers/crypto/caam/caamalg_desc.c | 46 ++++--
drivers/crypto/caam/caamalg_desc.h | 2 +-
drivers/crypto/caam/caamalg_qi.c | 222 +++++++++++++++----------
drivers/crypto/caam/caamalg_qi2.c | 316 ++++++++++++++++++++++++------------
drivers/crypto/caam/caamhash.c | 113 ++++++++-----
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/common_if.c | 88 ++++++++++
drivers/crypto/caam/common_if.h | 19 +++
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 | 5 +-
drivers/crypto/caam/qi.c | 10 +-
drivers/crypto/caam/regs.h | 1 +
19 files changed, 851 insertions(+), 400 deletions(-)
create mode 100644 drivers/crypto/caam/common_if.c
create mode 100644 drivers/crypto/caam/common_if.h

--
2.1.0


2019-07-18 14:45:59

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 03/14] crypto: caam - update IV only when crypto operation succeeds

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

2019-07-18 14:46:02

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 04/14] crypto: caam - check key length

Check key length to solve the extra tests that expect -EINVAL to be
returned when the key size is not valid.

Validated AES keylen for skcipher and ahash.

The check_aes_keylen function is added in a common file, to be used
also for caam/qi and caam/qi2.

Signed-off-by: Iuliana Prodan <[email protected]>
---
drivers/crypto/caam/Makefile | 2 +-
drivers/crypto/caam/caamalg.c | 124 +++++++++++++++++++++++++-------
drivers/crypto/caam/caamalg_qi.c | 128 ++++++++++++++++++++++++++-------
drivers/crypto/caam/caamalg_qi2.c | 147 +++++++++++++++++++++++++++++---------
drivers/crypto/caam/caamhash.c | 11 +++
drivers/crypto/caam/common_if.c | 31 ++++++++
drivers/crypto/caam/common_if.h | 13 ++++
7 files changed, 367 insertions(+), 89 deletions(-)
create mode 100644 drivers/crypto/caam/common_if.c
create mode 100644 drivers/crypto/caam/common_if.h

diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
index 9ab4e81..7edd697 100644
--- a/drivers/crypto/caam/Makefile
+++ b/drivers/crypto/caam/Makefile
@@ -8,7 +8,7 @@ endif

ccflags-y += -DVERSION=\"\"

-obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_COMMON) += error.o
+obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_COMMON) += error.o common_if.o
obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM) += caam.o
obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_JR) += caam_jr.o
obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_DESC) += caamalg_desc.o
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 28d55a0..6ac59b1 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -53,6 +53,7 @@
#include "desc_constr.h"
#include "jr.h"
#include "error.h"
+#include "common_if.h"
#include "sg_sw_sec4.h"
#include "key_gen.h"
#include "caamalg_desc.h"
@@ -667,6 +668,13 @@ static int gcm_setkey(struct crypto_aead *aead,
{
struct caam_ctx *ctx = crypto_aead_ctx(aead);
struct device *jrdev = ctx->jrdev;
+ int err;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }

print_hex_dump_debug("key in @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
@@ -683,10 +691,17 @@ static int rfc4106_setkey(struct crypto_aead *aead,
{
struct caam_ctx *ctx = crypto_aead_ctx(aead);
struct device *jrdev = ctx->jrdev;
+ int err;

if (keylen < 4)
return -EINVAL;

+ err = check_aes_keylen(keylen - 4);
+ if (err) {
+ crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
print_hex_dump_debug("key in @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);

@@ -707,10 +722,17 @@ static int rfc4543_setkey(struct crypto_aead *aead,
{
struct caam_ctx *ctx = crypto_aead_ctx(aead);
struct device *jrdev = ctx->jrdev;
+ int err;

if (keylen < 4)
return -EINVAL;

+ err = check_aes_keylen(keylen - 4);
+ if (err) {
+ crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
print_hex_dump_debug("key in @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);

@@ -727,7 +749,7 @@ static int rfc4543_setkey(struct crypto_aead *aead,
}

static int skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
- unsigned int keylen)
+ unsigned int keylen, const u32 ctx1_iv_off)
{
struct caam_ctx *ctx = crypto_skcipher_ctx(skcipher);
struct caam_skcipher_alg *alg =
@@ -736,30 +758,10 @@ static int skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
struct device *jrdev = ctx->jrdev;
unsigned int ivsize = crypto_skcipher_ivsize(skcipher);
u32 *desc;
- u32 ctx1_iv_off = 0;
- const bool ctr_mode = ((ctx->cdata.algtype & OP_ALG_AAI_MASK) ==
- OP_ALG_AAI_CTR_MOD128);
const bool is_rfc3686 = alg->caam.rfc3686;

print_hex_dump_debug("key in @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
- /*
- * AES-CTR needs to load IV in CONTEXT1 reg
- * at an offset of 128bits (16bytes)
- * CONTEXT1[255:128] = IV
- */
- if (ctr_mode)
- ctx1_iv_off = 16;
-
- /*
- * RFC3686 specific:
- * | CONTEXT1[255:128] = {NONCE, IV, COUNTER}
- * | *key = {KEY, NONCE}
- */
- if (is_rfc3686) {
- ctx1_iv_off = 16 + CTR_RFC3686_NONCE_SIZE;
- keylen -= CTR_RFC3686_NONCE_SIZE;
- }

ctx->cdata.keylen = keylen;
ctx->cdata.key_virt = key;
@@ -782,6 +784,74 @@ static int skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
return 0;
}

+static int aes_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ int err;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, 0);
+}
+
+static int rfc3686_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ u32 ctx1_iv_off;
+ int err;
+
+ /*
+ * RFC3686 specific:
+ * | CONTEXT1[255:128] = {NONCE, IV, COUNTER}
+ * | *key = {KEY, NONCE}
+ */
+ ctx1_iv_off = 16 + CTR_RFC3686_NONCE_SIZE;
+ keylen -= CTR_RFC3686_NONCE_SIZE;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, ctx1_iv_off);
+}
+
+static int ctr_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ u32 ctx1_iv_off;
+ int err;
+
+ /*
+ * AES-CTR needs to load IV in CONTEXT1 reg
+ * at an offset of 128bits (16bytes)
+ * CONTEXT1[255:128] = IV
+ */
+ ctx1_iv_off = 16;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, ctx1_iv_off);
+}
+
+static int arc4_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ return skcipher_setkey(skcipher, key, keylen, 0);
+}
+
static int des_skcipher_setkey(struct crypto_skcipher *skcipher,
const u8 *key, unsigned int keylen)
{
@@ -800,7 +870,7 @@ static int des_skcipher_setkey(struct crypto_skcipher *skcipher,
return -EINVAL;
}

- return skcipher_setkey(skcipher, key, keylen);
+ return skcipher_setkey(skcipher, key, keylen, 0);
}

static int xts_skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
@@ -1880,7 +1950,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "cbc-aes-caam",
.cra_blocksize = AES_BLOCK_SIZE,
},
- .setkey = skcipher_setkey,
+ .setkey = aes_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = AES_MIN_KEY_SIZE,
@@ -1928,7 +1998,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "ctr-aes-caam",
.cra_blocksize = 1,
},
- .setkey = skcipher_setkey,
+ .setkey = ctr_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = AES_MIN_KEY_SIZE,
@@ -1946,7 +2016,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "rfc3686-ctr-aes-caam",
.cra_blocksize = 1,
},
- .setkey = skcipher_setkey,
+ .setkey = rfc3686_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = AES_MIN_KEY_SIZE +
@@ -2000,7 +2070,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "ecb-aes-caam",
.cra_blocksize = AES_BLOCK_SIZE,
},
- .setkey = skcipher_setkey,
+ .setkey = aes_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = AES_MIN_KEY_SIZE,
@@ -2030,7 +2100,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "ecb-arc4-caam",
.cra_blocksize = ARC4_BLOCK_SIZE,
},
- .setkey = skcipher_setkey,
+ .setkey = arc4_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = ARC4_MIN_KEY_SIZE,
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index 66531d6..46097e3 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -385,6 +385,12 @@ static int gcm_setkey(struct crypto_aead *aead,
struct device *jrdev = ctx->jrdev;
int ret;

+ ret = check_aes_keylen(keylen);
+ if (ret) {
+ crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return ret;
+ }
+
print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);

@@ -483,6 +489,12 @@ static int rfc4106_setkey(struct crypto_aead *aead,
if (keylen < 4)
return -EINVAL;

+ ret = check_aes_keylen(keylen - 4);
+ if (ret) {
+ crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return ret;
+ }
+
print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);

@@ -585,6 +597,12 @@ static int rfc4543_setkey(struct crypto_aead *aead,
if (keylen < 4)
return -EINVAL;

+ ret = check_aes_keylen(keylen - 4);
+ if (ret) {
+ crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return ret;
+ }
+
print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);

@@ -624,7 +642,7 @@ static int rfc4543_setkey(struct crypto_aead *aead,
}

static int skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
- unsigned int keylen)
+ unsigned int keylen, const u32 ctx1_iv_off)
{
struct caam_ctx *ctx = crypto_skcipher_ctx(skcipher);
struct caam_skcipher_alg *alg =
@@ -632,33 +650,12 @@ static int skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
skcipher);
struct device *jrdev = ctx->jrdev;
unsigned int ivsize = crypto_skcipher_ivsize(skcipher);
- u32 ctx1_iv_off = 0;
- const bool ctr_mode = ((ctx->cdata.algtype & OP_ALG_AAI_MASK) ==
- OP_ALG_AAI_CTR_MOD128);
const bool is_rfc3686 = alg->caam.rfc3686;
int ret = 0;

print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);

- /*
- * AES-CTR needs to load IV in CONTEXT1 reg
- * at an offset of 128bits (16bytes)
- * CONTEXT1[255:128] = IV
- */
- if (ctr_mode)
- ctx1_iv_off = 16;
-
- /*
- * RFC3686 specific:
- * | CONTEXT1[255:128] = {NONCE, IV, COUNTER}
- * | *key = {KEY, NONCE}
- */
- if (is_rfc3686) {
- ctx1_iv_off = 16 + CTR_RFC3686_NONCE_SIZE;
- keylen -= CTR_RFC3686_NONCE_SIZE;
- }
-
ctx->cdata.keylen = keylen;
ctx->cdata.key_virt = key;
ctx->cdata.key_inline = true;
@@ -694,11 +691,88 @@ static int skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
return -EINVAL;
}

+static int aes_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ int err;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, 0);
+}
+
+static int rfc3686_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ u32 ctx1_iv_off;
+ int err;
+
+ /*
+ * RFC3686 specific:
+ * | CONTEXT1[255:128] = {NONCE, IV, COUNTER}
+ * | *key = {KEY, NONCE}
+ */
+ ctx1_iv_off = 16 + CTR_RFC3686_NONCE_SIZE;
+ keylen -= CTR_RFC3686_NONCE_SIZE;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, ctx1_iv_off);
+}
+
+static int ctr_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ u32 ctx1_iv_off;
+ int err;
+
+ /*
+ * AES-CTR needs to load IV in CONTEXT1 reg
+ * at an offset of 128bits (16bytes)
+ * CONTEXT1[255:128] = IV
+ */
+ ctx1_iv_off = 16;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, ctx1_iv_off);
+}
+
static int des3_skcipher_setkey(struct crypto_skcipher *skcipher,
const u8 *key, unsigned int keylen)
{
return unlikely(des3_verify_key(skcipher, key)) ?:
- skcipher_setkey(skcipher, key, keylen);
+ skcipher_setkey(skcipher, key, keylen, 0);
+}
+
+static int des_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ u32 tmp[DES_EXPKEY_WORDS];
+
+ if (!des_ekey(tmp, key) && (crypto_skcipher_get_flags(skcipher) &
+ CRYPTO_TFM_REQ_FORBID_WEAK_KEYS)) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_WEAK_KEY);
+ return -EINVAL;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, 0);
}

static int xts_skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
@@ -1405,7 +1479,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "cbc-aes-caam-qi",
.cra_blocksize = AES_BLOCK_SIZE,
},
- .setkey = skcipher_setkey,
+ .setkey = aes_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = AES_MIN_KEY_SIZE,
@@ -1437,7 +1511,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "cbc-des-caam-qi",
.cra_blocksize = DES_BLOCK_SIZE,
},
- .setkey = skcipher_setkey,
+ .setkey = des_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = DES_KEY_SIZE,
@@ -1453,7 +1527,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "ctr-aes-caam-qi",
.cra_blocksize = 1,
},
- .setkey = skcipher_setkey,
+ .setkey = ctr_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = AES_MIN_KEY_SIZE,
@@ -1471,7 +1545,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "rfc3686-ctr-aes-caam-qi",
.cra_blocksize = 1,
},
- .setkey = skcipher_setkey,
+ .setkey = rfc3686_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = AES_MIN_KEY_SIZE +
diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c
index bc370af..da4abf1 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -731,7 +731,13 @@ static int gcm_setkey(struct crypto_aead *aead,
{
struct caam_ctx *ctx = crypto_aead_ctx(aead);
struct device *dev = ctx->dev;
+ int ret;

+ ret = check_aes_keylen(keylen);
+ if (ret) {
+ crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return ret;
+ }
print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);

@@ -817,10 +823,17 @@ static int rfc4106_setkey(struct crypto_aead *aead,
{
struct caam_ctx *ctx = crypto_aead_ctx(aead);
struct device *dev = ctx->dev;
+ int ret;

if (keylen < 4)
return -EINVAL;

+ ret = check_aes_keylen(keylen - 4);
+ if (ret) {
+ crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return ret;
+ }
+
print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);

@@ -911,10 +924,17 @@ static int rfc4543_setkey(struct crypto_aead *aead,
{
struct caam_ctx *ctx = crypto_aead_ctx(aead);
struct device *dev = ctx->dev;
+ int ret;

if (keylen < 4)
return -EINVAL;

+ ret = check_aes_keylen(keylen - 4);
+ if (ret) {
+ crypto_aead_set_flags(aead, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return ret;
+ }
+
print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);

@@ -931,7 +951,7 @@ static int rfc4543_setkey(struct crypto_aead *aead,
}

static int skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
- unsigned int keylen)
+ unsigned int keylen, const u32 ctx1_iv_off)
{
struct caam_ctx *ctx = crypto_skcipher_ctx(skcipher);
struct caam_skcipher_alg *alg =
@@ -941,34 +961,11 @@ static int skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
struct caam_flc *flc;
unsigned int ivsize = crypto_skcipher_ivsize(skcipher);
u32 *desc;
- u32 ctx1_iv_off = 0;
- const bool ctr_mode = ((ctx->cdata.algtype & OP_ALG_AAI_MASK) ==
- OP_ALG_AAI_CTR_MOD128) &&
- ((ctx->cdata.algtype & OP_ALG_ALGSEL_MASK) !=
- OP_ALG_ALGSEL_CHACHA20);
const bool is_rfc3686 = alg->caam.rfc3686;

print_hex_dump_debug("key in @" __stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);

- /*
- * AES-CTR needs to load IV in CONTEXT1 reg
- * at an offset of 128bits (16bytes)
- * CONTEXT1[255:128] = IV
- */
- if (ctr_mode)
- ctx1_iv_off = 16;
-
- /*
- * RFC3686 specific:
- * | CONTEXT1[255:128] = {NONCE, IV, COUNTER}
- * | *key = {KEY, NONCE}
- */
- if (is_rfc3686) {
- ctx1_iv_off = 16 + CTR_RFC3686_NONCE_SIZE;
- keylen -= CTR_RFC3686_NONCE_SIZE;
- }
-
ctx->cdata.keylen = keylen;
ctx->cdata.key_virt = key;
ctx->cdata.key_inline = true;
@@ -996,11 +993,93 @@ static int skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
return 0;
}

-static int des3_skcipher_setkey(struct crypto_skcipher *skcipher,
- const u8 *key, unsigned int keylen)
+static int aes_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ int err;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, 0);
+}
+
+static int rfc3686_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
{
- return unlikely(des3_verify_key(skcipher, key)) ?:
- skcipher_setkey(skcipher, key, keylen);
+ u32 ctx1_iv_off;
+ int err;
+
+ /*
+ * RFC3686 specific:
+ * | CONTEXT1[255:128] = {NONCE, IV, COUNTER}
+ * | *key = {KEY, NONCE}
+ */
+ ctx1_iv_off = 16 + CTR_RFC3686_NONCE_SIZE;
+ keylen -= CTR_RFC3686_NONCE_SIZE;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, ctx1_iv_off);
+}
+
+static int ctr_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ u32 ctx1_iv_off;
+ int err;
+
+ /*
+ * AES-CTR needs to load IV in CONTEXT1 reg
+ * at an offset of 128bits (16bytes)
+ * CONTEXT1[255:128] = IV
+ */
+ ctx1_iv_off = 16;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, ctx1_iv_off);
+}
+
+static int chacha20_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ return skcipher_setkey(skcipher, key, keylen, 0);
+}
+
+static int des_skcipher_setkey(struct crypto_skcipher *skcipher,
+ const u8 *key, unsigned int keylen)
+{
+ u32 tmp[DES3_EDE_EXPKEY_WORDS];
+ struct crypto_tfm *tfm = crypto_skcipher_tfm(skcipher);
+
+ if (keylen == DES3_EDE_KEY_SIZE &&
+ __des3_ede_setkey(tmp, &tfm->crt_flags, key, DES3_EDE_KEY_SIZE)) {
+ return -EINVAL;
+ }
+
+ if (!des_ekey(tmp, key) && (crypto_skcipher_get_flags(skcipher) &
+ CRYPTO_TFM_REQ_FORBID_WEAK_KEYS)) {
+ crypto_skcipher_set_flags(skcipher,
+ CRYPTO_TFM_RES_WEAK_KEY);
+ return -EINVAL;
+ }
+
+ return skcipher_setkey(skcipher, key, keylen, 0);
}

static int xts_skcipher_setkey(struct crypto_skcipher *skcipher, const u8 *key,
@@ -1534,7 +1613,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "cbc-aes-caam-qi2",
.cra_blocksize = AES_BLOCK_SIZE,
},
- .setkey = skcipher_setkey,
+ .setkey = aes_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = AES_MIN_KEY_SIZE,
@@ -1550,7 +1629,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "cbc-3des-caam-qi2",
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
},
- .setkey = des3_skcipher_setkey,
+ .setkey = des_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = DES3_EDE_KEY_SIZE,
@@ -1566,7 +1645,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "cbc-des-caam-qi2",
.cra_blocksize = DES_BLOCK_SIZE,
},
- .setkey = skcipher_setkey,
+ .setkey = des_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = DES_KEY_SIZE,
@@ -1582,7 +1661,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "ctr-aes-caam-qi2",
.cra_blocksize = 1,
},
- .setkey = skcipher_setkey,
+ .setkey = ctr_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = AES_MIN_KEY_SIZE,
@@ -1600,7 +1679,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "rfc3686-ctr-aes-caam-qi2",
.cra_blocksize = 1,
},
- .setkey = skcipher_setkey,
+ .setkey = rfc3686_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = AES_MIN_KEY_SIZE +
@@ -1639,7 +1718,7 @@ static struct caam_skcipher_alg driver_algs[] = {
.cra_driver_name = "chacha20-caam-qi2",
.cra_blocksize = 1,
},
- .setkey = skcipher_setkey,
+ .setkey = chacha20_skcipher_setkey,
.encrypt = skcipher_encrypt,
.decrypt = skcipher_decrypt,
.min_keysize = CHACHA_KEY_SIZE,
diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 73abefa..2ec4bad 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -62,6 +62,7 @@
#include "desc_constr.h"
#include "jr.h"
#include "error.h"
+#include "common_if.h"
#include "sg_sw_sec4.h"
#include "key_gen.h"
#include "caamhash_desc.h"
@@ -501,6 +502,9 @@ static int axcbc_setkey(struct crypto_ahash *ahash, const u8 *key,
struct caam_hash_ctx *ctx = crypto_ahash_ctx(ahash);
struct device *jrdev = ctx->jrdev;

+ if (keylen != AES_KEYSIZE_128)
+ return -EINVAL;
+
memcpy(ctx->key, key, keylen);
dma_sync_single_for_device(jrdev, ctx->key_dma, keylen, DMA_TO_DEVICE);
ctx->adata.keylen = keylen;
@@ -515,6 +519,13 @@ static int acmac_setkey(struct crypto_ahash *ahash, const u8 *key,
unsigned int keylen)
{
struct caam_hash_ctx *ctx = crypto_ahash_ctx(ahash);
+ int err;
+
+ err = check_aes_keylen(keylen);
+ if (err) {
+ crypto_ahash_set_flags(ahash, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return err;
+ }

/* key is immediate data for all cmac shared descriptors */
ctx->adata.key_virt = key;
diff --git a/drivers/crypto/caam/common_if.c b/drivers/crypto/caam/common_if.c
new file mode 100644
index 0000000..859d4b4
--- /dev/null
+++ b/drivers/crypto/caam/common_if.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * CAAM Common Location for caam/jr, caam/qi, caam/qi2
+ *
+ * Copyright 2019 NXP
+ */
+
+#include "compat.h"
+#include "common_if.h"
+
+/*
+ * validate key length for AES algorithms
+ */
+int check_aes_keylen(unsigned int keylen)
+{
+ switch (keylen) {
+ case AES_KEYSIZE_128:
+ case AES_KEYSIZE_192:
+ case AES_KEYSIZE_256:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(check_aes_keylen);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("FSL CAAM drivers common location");
+MODULE_AUTHOR("NXP Semiconductors");
diff --git a/drivers/crypto/caam/common_if.h b/drivers/crypto/caam/common_if.h
new file mode 100644
index 0000000..6964ba3
--- /dev/null
+++ b/drivers/crypto/caam/common_if.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * CAAM Common Location for caam/jr, caam/qi, caam/qi2
+ *
+ * Copyright 2019 NXP
+ */
+
+#ifndef CAAM_COMMON_LOCATION_H
+#define CAAM_COMMON_LOCATION_H
+
+int check_aes_keylen(unsigned int keylen);
+
+#endif /* CAAM_COMMON_LOCATION_H */
--
2.1.0

2019-07-18 14:46:10

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 13/14] crypto: caam - unregister algorithm only if the registration succeeded

To know if a registration succeeded added a new struct,
caam_akcipher_alg, that keeps, also, the registration status.
This status is updated in caam_pkc_init and verified in
caam_pkc_exit to unregister an algorithm.

Signed-off-by: Iuliana Prodan <[email protected]>
---
drivers/crypto/caam/caampkc.c | 49 ++++++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index cfdf7a2..a669688 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -35,6 +35,11 @@ static u8 *zero_buffer;
*/
static bool init_done;

+struct caam_akcipher_alg {
+ struct akcipher_alg akcipher;
+ bool registered;
+};
+
static void rsa_io_unmap(struct device *dev, struct rsa_edesc *edesc,
struct akcipher_request *req)
{
@@ -1063,22 +1068,24 @@ static void caam_rsa_exit_tfm(struct crypto_akcipher *tfm)
caam_jr_free(ctx->dev);
}

-static struct akcipher_alg caam_rsa = {
- .encrypt = caam_rsa_enc,
- .decrypt = caam_rsa_dec,
- .set_pub_key = caam_rsa_set_pub_key,
- .set_priv_key = caam_rsa_set_priv_key,
- .max_size = caam_rsa_max_size,
- .init = caam_rsa_init_tfm,
- .exit = caam_rsa_exit_tfm,
- .reqsize = sizeof(struct caam_rsa_req_ctx),
- .base = {
- .cra_name = "rsa",
- .cra_driver_name = "rsa-caam",
- .cra_priority = 3000,
- .cra_module = THIS_MODULE,
- .cra_ctxsize = sizeof(struct caam_rsa_ctx),
- },
+static struct caam_akcipher_alg caam_rsa = {
+ .akcipher = {
+ .encrypt = caam_rsa_enc,
+ .decrypt = caam_rsa_dec,
+ .set_pub_key = caam_rsa_set_pub_key,
+ .set_priv_key = caam_rsa_set_priv_key,
+ .max_size = caam_rsa_max_size,
+ .init = caam_rsa_init_tfm,
+ .exit = caam_rsa_exit_tfm,
+ .reqsize = sizeof(struct caam_rsa_req_ctx),
+ .base = {
+ .cra_name = "rsa",
+ .cra_driver_name = "rsa-caam",
+ .cra_priority = 3000,
+ .cra_module = THIS_MODULE,
+ .cra_ctxsize = sizeof(struct caam_rsa_ctx),
+ },
+ }
};

/* Public Key Cryptography module initialization handler */
@@ -1106,13 +1113,15 @@ int caam_pkc_init(struct device *ctrldev)
if (!zero_buffer)
return -ENOMEM;

- err = crypto_register_akcipher(&caam_rsa);
+ err = crypto_register_akcipher(&caam_rsa.akcipher);
+
if (err) {
kfree(zero_buffer);
dev_warn(ctrldev, "%s alg registration failed\n",
- caam_rsa.base.cra_driver_name);
+ caam_rsa.akcipher.base.cra_driver_name);
} else {
init_done = true;
+ caam_rsa.registered = true;
dev_info(ctrldev, "caam pkc algorithms registered in /proc/crypto\n");
}

@@ -1124,6 +1133,8 @@ void caam_pkc_exit(void)
if (!init_done)
return;

+ if (caam_rsa.registered)
+ crypto_unregister_akcipher(&caam_rsa.akcipher);
+
kfree(zero_buffer);
- crypto_unregister_akcipher(&caam_rsa);
}
--
2.1.0

2019-07-18 14:46:11

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 10/14] crypto: caam - fix DKP for certain key lengths

From: Horia Geantă <[email protected]>

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.

Signed-off-by: Horia Geantă <[email protected]>
---
drivers/crypto/caam/caamalg.c | 42 +++++++----------------
drivers/crypto/caam/caamalg_qi.c | 42 +++++++----------------
drivers/crypto/caam/caamalg_qi2.c | 70 ++++++++++++++++++++++++++++-----------
drivers/crypto/caam/caamhash.c | 54 +++++++++++++++++++++---------
drivers/crypto/caam/desc_constr.h | 24 ++++++++++----
5 files changed, 131 insertions(+), 101 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 9a7bb06..22d9fa0 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -206,6 +206,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;

@@ -222,16 +234,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);

@@ -254,16 +256,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);

@@ -288,16 +280,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 b2dd4f3..c7cebd6 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -106,6 +106,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;

@@ -119,16 +131,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);

@@ -144,16 +146,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);

@@ -172,16 +164,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 285e3c9..6541b10 100644
--- 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,14 +2991,18 @@ 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
+ * @key_dma: I/O virtual address of the key
* @dev: dpseci device
* @ctx_len: size of Context Register
* @adata: hashing algorithm details
*/
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];
+ dma_addr_t key_dma;
struct device *dev;
int ctx_len;
struct alginfo adata;
@@ -3306,6 +3302,20 @@ 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) {
+ ctx->adata.key_dma = ctx->key_dma;
+ memcpy(ctx->key, key, keylen);
+ dma_sync_single_for_device(ctx->dev, ctx->key_dma,
+ ctx->adata.keylen_pad,
+ DMA_TO_DEVICE);
+ }
+
ret = ahash_set_sh_desc(ahash);
kfree(hashed_key);
return ret;
@@ -4536,11 +4546,27 @@ static int caam_hash_cra_init(struct crypto_tfm *tfm)

ctx->dev = caam_hash->dev;

+ if (alg->setkey) {
+ ctx->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->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->key_dma)
+ dma_unmap_single_attrs(ctx->dev, ctx->key_dma,
+ ARRAY_SIZE(ctx->key),
+ DMA_TO_DEVICE,
+ DMA_ATTR_SKIP_CPU_SYNC);
return -ENOMEM;
}

@@ -4566,6 +4592,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->key_dma)
+ dma_unmap_single_attrs(ctx->dev, ctx->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 14fdfa1..e89913b 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -98,6 +98,7 @@ struct caam_hash_ctx {
dma_addr_t sh_desc_digest_dma;
dma_addr_t key_dma;
enum dma_data_direction dir;
+ enum dma_data_direction key_dir;
struct device *jrdev;
int ctx_len;
struct alginfo adata;
@@ -475,6 +476,19 @@ 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) {
+ ctx->adata.key_dma = ctx->key_dma;
+ dma_sync_single_for_device(ctx->jrdev, ctx->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 +1839,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->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->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 = DMA_TO_DEVICE;
+ } 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->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->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->key_dma,
ARRAY_SIZE(ctx->key),
- DMA_BIDIRECTIONAL,
+ ctx->key_dir,
DMA_ATTR_SKIP_CPU_SYNC);

caam_jr_free(ctx->jrdev);
@@ -1891,9 +1915,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->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 {
--
2.1.0

2019-07-18 14:46:30

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 12/14] crypto: caam - execute module exit point only if necessary

Commit 1b46c90c8e00 ("crypto: caam - convert top level drivers to libraries")
changed entry and exit points behavior for caamalg,
caamalg_qi, caamalg_qi2, caamhash, caampkc, caamrng.

For example, previously caam_pkc_init() and caam_pkc_exit() were
module entry/exit points. This means that if an error would happen
in caam_pkc_init(), then caam_pkc_exit() wouldn't have been called.
After the mentioned commit, caam_pkc_init() and caam_pkc_exit()
are manually called - from jr.c. caam_pkc_exit() is called
unconditionally, even if caam_pkc_init() failed.

Added a global variable to keep the status of the algorithm
registration and free of resources.
The exit point of caampkc/caamrng module is executed only if the
registration was successful. Therefore we avoid double free of
resources in case the algorithm registration failed.

Fixes: 1b46c90c8e00 ("crypto: caam - convert top level drivers to libraries")
Signed-off-by: Iuliana Prodan <[email protected]>
---
drivers/crypto/caam/caampkc.c | 11 +++++++++++
drivers/crypto/caam/caamrng.c | 14 +++++++++++++-
2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index 574428c7..cfdf7a2 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -29,6 +29,12 @@
/* buffer filled with zeros, used for padding */
static u8 *zero_buffer;

+/*
+ * variable used to avoid double free of resources in case
+ * algorithm registration was unsuccessful
+ */
+static bool init_done;
+
static void rsa_io_unmap(struct device *dev, struct rsa_edesc *edesc,
struct akcipher_request *req)
{
@@ -1081,6 +1087,7 @@ int caam_pkc_init(struct device *ctrldev)
struct caam_drv_private *priv = dev_get_drvdata(ctrldev);
u32 pk_inst;
int err;
+ init_done = false;

/* Determine public key hardware accelerator presence. */
if (priv->era < 10)
@@ -1105,6 +1112,7 @@ int caam_pkc_init(struct device *ctrldev)
dev_warn(ctrldev, "%s alg registration failed\n",
caam_rsa.base.cra_driver_name);
} else {
+ init_done = true;
dev_info(ctrldev, "caam pkc algorithms registered in /proc/crypto\n");
}

@@ -1113,6 +1121,9 @@ int caam_pkc_init(struct device *ctrldev)

void caam_pkc_exit(void)
{
+ if (!init_done)
+ return;
+
kfree(zero_buffer);
crypto_unregister_akcipher(&caam_rsa);
}
diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index 54c32d5..7fbda1b 100644
--- a/drivers/crypto/caam/caamrng.c
+++ b/drivers/crypto/caam/caamrng.c
@@ -80,6 +80,12 @@ struct caam_rng_ctx {

static struct caam_rng_ctx *rng_ctx;

+/*
+ * Variable used to avoid double free of resources in case
+ * algorithm registration was unsuccessful
+ */
+static bool init_done;
+
static inline void rng_unmap_buf(struct device *jrdev, struct buf_data *bd)
{
if (bd->addr)
@@ -296,6 +302,9 @@ static struct hwrng caam_rng = {

void caam_rng_exit(void)
{
+ if (!init_done)
+ return;
+
caam_jr_free(rng_ctx->jrdev);
hwrng_unregister(&caam_rng);
kfree(rng_ctx);
@@ -307,6 +316,7 @@ int caam_rng_init(struct device *ctrldev)
u32 rng_inst;
struct caam_drv_private *priv = dev_get_drvdata(ctrldev);
int err;
+ init_done = false;

/* Check for an instantiated RNG before registration */
if (priv->era < 10)
@@ -335,8 +345,10 @@ int caam_rng_init(struct device *ctrldev)
dev_info(dev, "registering rng-caam\n");

err = hwrng_register(&caam_rng);
- if (!err)
+ if (!err) {
+ init_done = true;
return err;
+ }

free_rng_ctx:
kfree(rng_ctx);
--
2.1.0

2019-07-18 14:46:38

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 14/14] crypto: caam - change return value in case CAAM has no MDHA

To be consistent with other CAAM modules, caamhash should return 0
instead of -ENODEV in case CAAM has no MDHA.

Based on commit 1b46c90c8e00 ("crypto: caam - convert top level drivers to libraries")
the value returned by entry point is never checked and
the exit point is always executed.

Signed-off-by: Iuliana Prodan <[email protected]>
---
drivers/crypto/caam/caamhash.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index e89913b..d4e047f 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -2008,7 +2008,7 @@ int caam_algapi_hash_init(struct device *ctrldev)
* is not present.
*/
if (!md_inst)
- return -ENODEV;
+ return 0;

/* Limit digest size based on LP256 */
if (md_vid == CHA_VER_VID_MD_LP256)
--
2.1.0

2019-07-18 14:46:38

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 09/14] crypto: caam - keep both virtual and dma key addresses

From: Horia Geantă <[email protected]>

Update alginfo struct to keep both virtual and dma key addresses,
so that descriptors have them at hand.
One example where this is needed is in the xcbc(aes) shared descriptors,
which are updated in current patch.
Another example is the upcoming fix for DKP.

Signed-off-by: Horia Geantă <[email protected]>
---
drivers/crypto/caam/caamhash.c | 26 ++++++++++++--------------
drivers/crypto/caam/caamhash_desc.c | 5 ++---
drivers/crypto/caam/caamhash_desc.h | 2 +-
drivers/crypto/caam/desc_constr.h | 10 ++++------
4 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 2ec4bad..14fdfa1 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -283,13 +283,10 @@ static int axcbc_set_sh_desc(struct crypto_ahash *ahash)
struct device *jrdev = ctx->jrdev;
u32 *desc;

- /* key is loaded from memory for UPDATE and FINALIZE states */
- ctx->adata.key_dma = ctx->key_dma;
-
/* shared descriptor for ahash_update */
desc = ctx->sh_desc_update;
cnstr_shdsc_sk_hash(desc, &ctx->adata, OP_ALG_AS_UPDATE,
- ctx->ctx_len, ctx->ctx_len, 0);
+ ctx->ctx_len, ctx->ctx_len);
dma_sync_single_for_device(jrdev, ctx->sh_desc_update_dma,
desc_bytes(desc), ctx->dir);
print_hex_dump_debug("axcbc update shdesc@" __stringify(__LINE__)" : ",
@@ -299,20 +296,17 @@ static int axcbc_set_sh_desc(struct crypto_ahash *ahash)
/* shared descriptor for ahash_{final,finup} */
desc = ctx->sh_desc_fin;
cnstr_shdsc_sk_hash(desc, &ctx->adata, OP_ALG_AS_FINALIZE,
- digestsize, ctx->ctx_len, 0);
+ digestsize, ctx->ctx_len);
dma_sync_single_for_device(jrdev, ctx->sh_desc_fin_dma,
desc_bytes(desc), ctx->dir);
print_hex_dump_debug("axcbc finup shdesc@" __stringify(__LINE__)" : ",
DUMP_PREFIX_ADDRESS, 16, 4, desc, desc_bytes(desc),
1);

- /* key is immediate data for INIT and INITFINAL states */
- ctx->adata.key_virt = ctx->key;
-
/* shared descriptor for first invocation of ahash_update */
desc = ctx->sh_desc_update_first;
cnstr_shdsc_sk_hash(desc, &ctx->adata, OP_ALG_AS_INIT, ctx->ctx_len,
- ctx->ctx_len, ctx->key_dma);
+ ctx->ctx_len);
dma_sync_single_for_device(jrdev, ctx->sh_desc_update_first_dma,
desc_bytes(desc), ctx->dir);
print_hex_dump_debug("axcbc update first shdesc@" __stringify(__LINE__)
@@ -322,7 +316,7 @@ static int axcbc_set_sh_desc(struct crypto_ahash *ahash)
/* shared descriptor for ahash_digest */
desc = ctx->sh_desc_digest;
cnstr_shdsc_sk_hash(desc, &ctx->adata, OP_ALG_AS_INITFINAL,
- digestsize, ctx->ctx_len, 0);
+ digestsize, ctx->ctx_len);
dma_sync_single_for_device(jrdev, ctx->sh_desc_digest_dma,
desc_bytes(desc), ctx->dir);
print_hex_dump_debug("axcbc digest shdesc@" __stringify(__LINE__)" : ",
@@ -341,7 +335,7 @@ static int acmac_set_sh_desc(struct crypto_ahash *ahash)
/* shared descriptor for ahash_update */
desc = ctx->sh_desc_update;
cnstr_shdsc_sk_hash(desc, &ctx->adata, OP_ALG_AS_UPDATE,
- ctx->ctx_len, ctx->ctx_len, 0);
+ ctx->ctx_len, ctx->ctx_len);
dma_sync_single_for_device(jrdev, ctx->sh_desc_update_dma,
desc_bytes(desc), ctx->dir);
print_hex_dump_debug("acmac update shdesc@" __stringify(__LINE__)" : ",
@@ -351,7 +345,7 @@ static int acmac_set_sh_desc(struct crypto_ahash *ahash)
/* shared descriptor for ahash_{final,finup} */
desc = ctx->sh_desc_fin;
cnstr_shdsc_sk_hash(desc, &ctx->adata, OP_ALG_AS_FINALIZE,
- digestsize, ctx->ctx_len, 0);
+ digestsize, ctx->ctx_len);
dma_sync_single_for_device(jrdev, ctx->sh_desc_fin_dma,
desc_bytes(desc), ctx->dir);
print_hex_dump_debug("acmac finup shdesc@" __stringify(__LINE__)" : ",
@@ -361,7 +355,7 @@ static int acmac_set_sh_desc(struct crypto_ahash *ahash)
/* shared descriptor for first invocation of ahash_update */
desc = ctx->sh_desc_update_first;
cnstr_shdsc_sk_hash(desc, &ctx->adata, OP_ALG_AS_INIT, ctx->ctx_len,
- ctx->ctx_len, 0);
+ ctx->ctx_len);
dma_sync_single_for_device(jrdev, ctx->sh_desc_update_first_dma,
desc_bytes(desc), ctx->dir);
print_hex_dump_debug("acmac update first shdesc@" __stringify(__LINE__)
@@ -371,7 +365,7 @@ static int acmac_set_sh_desc(struct crypto_ahash *ahash)
/* shared descriptor for ahash_digest */
desc = ctx->sh_desc_digest;
cnstr_shdsc_sk_hash(desc, &ctx->adata, OP_ALG_AS_INITFINAL,
- digestsize, ctx->ctx_len, 0);
+ digestsize, ctx->ctx_len);
dma_sync_single_for_device(jrdev, ctx->sh_desc_digest_dma,
desc_bytes(desc), ctx->dir);
print_hex_dump_debug("acmac digest shdesc@" __stringify(__LINE__)" : ",
@@ -508,6 +502,10 @@ static int axcbc_setkey(struct crypto_ahash *ahash, const u8 *key,
memcpy(ctx->key, key, keylen);
dma_sync_single_for_device(jrdev, ctx->key_dma, keylen, DMA_TO_DEVICE);
ctx->adata.keylen = keylen;
+ /* key is loaded from memory for UPDATE and FINALIZE states */
+ ctx->adata.key_dma = ctx->key_dma;
+ /* key is immediate data for INIT and INITFINAL states */
+ ctx->adata.key_virt = ctx->key;

print_hex_dump_debug("axcbc ctx.key@" __stringify(__LINE__)" : ",
DUMP_PREFIX_ADDRESS, 16, 4, ctx->key, keylen, 1);
diff --git a/drivers/crypto/caam/caamhash_desc.c b/drivers/crypto/caam/caamhash_desc.c
index 71d0183..78383d7 100644
--- a/drivers/crypto/caam/caamhash_desc.c
+++ b/drivers/crypto/caam/caamhash_desc.c
@@ -83,10 +83,9 @@ EXPORT_SYMBOL(cnstr_shdsc_ahash);
* @state: algorithm state OP_ALG_AS_{INIT, FINALIZE, INITFINALIZE, UPDATE}
* @digestsize: algorithm's digest size
* @ctx_len: size of Context Register
- * @key_dma: I/O Virtual Address of the key
*/
void cnstr_shdsc_sk_hash(u32 * const desc, struct alginfo *adata, u32 state,
- int digestsize, int ctx_len, dma_addr_t key_dma)
+ int digestsize, int ctx_len)
{
u32 *skip_key_load;

@@ -136,7 +135,7 @@ void cnstr_shdsc_sk_hash(u32 * const desc, struct alginfo *adata, u32 state,
LDST_SRCDST_BYTE_CONTEXT);
if (is_xcbc_aes(adata->algtype) && state == OP_ALG_AS_INIT)
/* Save K1 */
- append_fifo_store(desc, key_dma, adata->keylen,
+ append_fifo_store(desc, adata->key_dma, adata->keylen,
LDST_CLASS_1_CCB | FIFOST_TYPE_KEY_KEK);
}
EXPORT_SYMBOL(cnstr_shdsc_sk_hash);
diff --git a/drivers/crypto/caam/caamhash_desc.h b/drivers/crypto/caam/caamhash_desc.h
index 6947ee1..4f369b8 100644
--- a/drivers/crypto/caam/caamhash_desc.h
+++ b/drivers/crypto/caam/caamhash_desc.h
@@ -25,5 +25,5 @@ void cnstr_shdsc_ahash(u32 * const desc, struct alginfo *adata, u32 state,
int digestsize, int ctx_len, bool import_ctx, int era);

void cnstr_shdsc_sk_hash(u32 * const desc, struct alginfo *adata, u32 state,
- int digestsize, int ctx_len, dma_addr_t key_dma);
+ int digestsize, int ctx_len);
#endif /* _CAAMHASH_DESC_H_ */
diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h
index 5988a26..8154174 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -457,8 +457,8 @@ do { \
* functions where it is used.
* @keylen: length of the provided algorithm key, in bytes
* @keylen_pad: padded length of the provided algorithm key, in bytes
- * @key: address where algorithm key resides; virtual address if key_inline
- * is true, dma (bus) address if key_inline is false.
+ * @key_dma: dma (bus) address where algorithm key resides
+ * @key_virt: virtual address where algorithm key resides
* @key_inline: true - key can be inlined in the descriptor; false - key is
* referenced by the descriptor
*/
@@ -466,10 +466,8 @@ struct alginfo {
u32 algtype;
unsigned int keylen;
unsigned int keylen_pad;
- union {
- dma_addr_t key_dma;
- const void *key_virt;
- };
+ dma_addr_t key_dma;
+ const void *key_virt;
bool key_inline;
};

--
2.1.0

2019-07-18 14:46:50

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 08/14] crypto: caam - update rfc4106 sh desc to support zero length input

Update share descriptor for rfc4106 to skip instructions in case
cryptlen is zero. If no instructions are jumped the DECO hangs and a
timeout error is thrown.

Signed-off-by: Iuliana Prodan <[email protected]>
---
drivers/crypto/caam/caamalg_desc.c | 46 +++++++++++++++++++++++++-------------
drivers/crypto/caam/caamalg_desc.h | 2 +-
2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/drivers/crypto/caam/caamalg_desc.c b/drivers/crypto/caam/caamalg_desc.c
index 7253183..99f419a 100644
--- a/drivers/crypto/caam/caamalg_desc.c
+++ b/drivers/crypto/caam/caamalg_desc.c
@@ -843,13 +843,16 @@ EXPORT_SYMBOL(cnstr_shdsc_gcm_decap);
* @ivsize: initialization vector size
* @icvsize: integrity check value (ICV) size (truncated or full)
* @is_qi: true when called from caam/qi
+ *
+ * Input sequence: AAD | PTXT
+ * Output sequence: AAD | CTXT | ICV
+ * AAD length (assoclen), which includes the IV length, is available in Math3.
*/
void cnstr_shdsc_rfc4106_encap(u32 * const desc, struct alginfo *cdata,
unsigned int ivsize, unsigned int icvsize,
const bool is_qi)
{
- u32 *key_jump_cmd;
-
+ u32 *key_jump_cmd, *zero_cryptlen_jump_cmd, *skip_instructions;
init_sh_desc(desc, HDR_SHARE_SERIAL);

/* Skip key loading if it is loaded due to sharing */
@@ -890,26 +893,25 @@ void cnstr_shdsc_rfc4106_encap(u32 * const desc, struct alginfo *cdata,
}

append_math_sub_imm_u32(desc, VARSEQINLEN, REG3, IMM, ivsize);
- append_math_add(desc, VARSEQOUTLEN, ZERO, REG3, CAAM_CMD_SZ);
+ append_math_add(desc, VARSEQOUTLEN, REG0, REG3, CAAM_CMD_SZ);

- /* Read assoc data */
+ /* Skip AAD */
+ append_seq_fifo_store(desc, 0, FIFOST_TYPE_SKIP | FIFOLDST_VLF);
+
+ /* Read cryptlen and set this value into VARSEQOUTLEN */
+ append_math_sub(desc, VARSEQOUTLEN, SEQINLEN, REG3, CAAM_CMD_SZ);
+
+ /* If cryptlen is ZERO jump to AAD command */
+ zero_cryptlen_jump_cmd = append_jump(desc, JUMP_TEST_ALL |
+ JUMP_COND_MATH_Z);
+
+ /* Read AAD data */
append_seq_fifo_load(desc, 0, FIFOLD_CLASS_CLASS1 | FIFOLDST_VLF |
FIFOLD_TYPE_AAD | FIFOLD_TYPE_FLUSH1);

/* Skip IV */
append_seq_fifo_load(desc, ivsize, FIFOLD_CLASS_SKIP);
-
- /* Will read cryptlen bytes */
- append_math_sub(desc, VARSEQINLEN, SEQINLEN, REG0, CAAM_CMD_SZ);
-
- /* Workaround for erratum A-005473 (simultaneous SEQ FIFO skips) */
- append_seq_fifo_load(desc, 0, FIFOLD_CLASS_CLASS1 | FIFOLD_TYPE_MSG);
-
- /* Skip assoc data */
- append_seq_fifo_store(desc, 0, FIFOST_TYPE_SKIP | FIFOLDST_VLF);
-
- /* cryptlen = seqoutlen - assoclen */
- append_math_sub(desc, VARSEQOUTLEN, VARSEQINLEN, REG0, CAAM_CMD_SZ);
+ append_math_add(desc, VARSEQINLEN, VARSEQOUTLEN, REG0, CAAM_CMD_SZ);

/* Write encrypted data */
append_seq_fifo_store(desc, 0, FIFOST_TYPE_MESSAGE_DATA | FIFOLDST_VLF);
@@ -918,6 +920,18 @@ void cnstr_shdsc_rfc4106_encap(u32 * const desc, struct alginfo *cdata,
append_seq_fifo_load(desc, 0, FIFOLD_CLASS_CLASS1 | FIFOLDST_VLF |
FIFOLD_TYPE_MSG | FIFOLD_TYPE_LAST1);

+ /* Jump instructions to avoid double reading of AAD */
+ skip_instructions = append_jump(desc, JUMP_TEST_ALL);
+
+ /* There is no input data, cryptlen = 0 */
+ set_jump_tgt_here(desc, zero_cryptlen_jump_cmd);
+
+ /* Read AAD */
+ append_seq_fifo_load(desc, 0, FIFOLD_CLASS_CLASS1 | FIFOLDST_VLF |
+ FIFOLD_TYPE_AAD | FIFOLD_TYPE_LAST1);
+
+ set_jump_tgt_here(desc, skip_instructions);
+
/* Write ICV */
append_seq_store(desc, icvsize, LDST_CLASS_1_CCB |
LDST_SRCDST_BYTE_CONTEXT);
diff --git a/drivers/crypto/caam/caamalg_desc.h b/drivers/crypto/caam/caamalg_desc.h
index da4a4ee..a49fb53 100644
--- a/drivers/crypto/caam/caamalg_desc.h
+++ b/drivers/crypto/caam/caamalg_desc.h
@@ -31,7 +31,7 @@
#define DESC_QI_GCM_DEC_LEN (DESC_GCM_DEC_LEN + 3 * CAAM_CMD_SZ)

#define DESC_RFC4106_BASE (3 * CAAM_CMD_SZ)
-#define DESC_RFC4106_ENC_LEN (DESC_RFC4106_BASE + 13 * CAAM_CMD_SZ)
+#define DESC_RFC4106_ENC_LEN (DESC_RFC4106_BASE + 15 * CAAM_CMD_SZ)
#define DESC_RFC4106_DEC_LEN (DESC_RFC4106_BASE + 13 * CAAM_CMD_SZ)
#define DESC_QI_RFC4106_ENC_LEN (DESC_RFC4106_ENC_LEN + 5 * CAAM_CMD_SZ)
#define DESC_QI_RFC4106_DEC_LEN (DESC_RFC4106_DEC_LEN + 5 * CAAM_CMD_SZ)
--
2.1.0

2019-07-18 14:47:05

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 06/14] crypto: caam - check assoclen

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 ++--------
drivers/crypto/caam/common_if.c | 17 +++++++++++++++++
drivers/crypto/caam/common_if.h | 2 ++
5 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 6682e67..6b9937c 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -1605,10 +1605,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)
@@ -1682,10 +1679,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 5f9b14a..69cc657 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -1244,18 +1244,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 0b4de21..da3452b 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -1407,18 +1407,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)
diff --git a/drivers/crypto/caam/common_if.c b/drivers/crypto/caam/common_if.c
index fcf47e6..1291d3d 100644
--- a/drivers/crypto/caam/common_if.c
+++ b/drivers/crypto/caam/common_if.c
@@ -66,6 +66,23 @@ int check_rfc4106_authsize(unsigned int authsize)
}
EXPORT_SYMBOL(check_rfc4106_authsize);

+/*
+ * validate assoclen for RFC4106/RFC4543
+ */
+int check_ipsec_assoclen(unsigned int assoclen)
+{
+ switch (assoclen) {
+ case 16:
+ case 20:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(check_ipsec_assoclen);
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("FSL CAAM drivers common location");
MODULE_AUTHOR("NXP Semiconductors");
diff --git a/drivers/crypto/caam/common_if.h b/drivers/crypto/caam/common_if.h
index b17386a..61d5516 100644
--- a/drivers/crypto/caam/common_if.h
+++ b/drivers/crypto/caam/common_if.h
@@ -14,4 +14,6 @@ int check_gcm_authsize(unsigned int authsize);

int check_rfc4106_authsize(unsigned int authsize);

+int check_ipsec_assoclen(unsigned int assoclen);
+
#endif /* CAAM_COMMON_LOCATION_H */
--
2.1.0

2019-07-18 14:47:25

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 07/14] crypto: caam - check zero-length input

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]>
---
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 6b9937c..9a7bb06 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -1891,6 +1891,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))
@@ -1925,6 +1928,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 69cc657..b2dd4f3 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -1450,6 +1450,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 da3452b..285e3c9 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -1499,6 +1499,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))
@@ -1527,6 +1530,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

2019-07-18 14:47:25

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 05/14] crypto: caam - check authsize

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 | 14 ++++++++++++++
drivers/crypto/caam/caamalg_qi2.c | 14 ++++++++++++++
drivers/crypto/caam/common_if.c | 40 +++++++++++++++++++++++++++++++++++++++
drivers/crypto/caam/common_if.h | 4 ++++
5 files changed, 85 insertions(+)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 6ac59b1..6682e67 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -377,6 +377,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);
@@ -440,6 +445,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);
@@ -504,6 +514,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 46097e3..5f9b14a 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -18,6 +18,7 @@
#include "qi.h"
#include "jr.h"
#include "caamalg_desc.h"
+#include "common_if.h"

/*
* crypto alg
@@ -371,6 +372,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 +478,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);
@@ -581,6 +592,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 da4abf1..0b4de21 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -10,6 +10,7 @@
#include "dpseci_cmd.h"
#include "desc_constr.h"
#include "error.h"
+#include "common_if.h"
#include "sg_sw_sec4.h"
#include "sg_sw_qm2.h"
#include "key_gen.h"
@@ -719,6 +720,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 +817,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);
@@ -913,6 +924,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/common_if.c b/drivers/crypto/caam/common_if.c
index 859d4b4..fcf47e6 100644
--- a/drivers/crypto/caam/common_if.c
+++ b/drivers/crypto/caam/common_if.c
@@ -26,6 +26,46 @@ int check_aes_keylen(unsigned int keylen)
}
EXPORT_SYMBOL(check_aes_keylen);

+/*
+ * validate authentication tag for GCM
+ */
+int check_gcm_authsize(unsigned int authsize)
+{
+ switch (authsize) {
+ case 4:
+ case 8:
+ case 12:
+ case 13:
+ case 14:
+ case 15:
+ case 16:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(check_gcm_authsize);
+
+/*
+ * validate authentication tag for RFC4106
+ */
+int check_rfc4106_authsize(unsigned int authsize)
+{
+ switch (authsize) {
+ case 8:
+ case 12:
+ case 16:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(check_rfc4106_authsize);
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("FSL CAAM drivers common location");
MODULE_AUTHOR("NXP Semiconductors");
diff --git a/drivers/crypto/caam/common_if.h b/drivers/crypto/caam/common_if.h
index 6964ba3..b17386a 100644
--- a/drivers/crypto/caam/common_if.h
+++ b/drivers/crypto/caam/common_if.h
@@ -10,4 +10,8 @@

int check_aes_keylen(unsigned int keylen);

+int check_gcm_authsize(unsigned int authsize);
+
+int check_rfc4106_authsize(unsigned int authsize);
+
#endif /* CAAM_COMMON_LOCATION_H */
--
2.1.0

2019-07-18 14:47:34

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 01/14] crypto: caam/qi - fix error handling in ERN handler

From: Horia Geantă <[email protected]>

ERN handler calls the caam/qi frontend "done" callback with a status
of -EIO. This is incorrect, since the callback expects a status value
meaningful for the crypto engine - hence the cryptic messages
like the one below:
platform caam_qi: 15: unknown error source

Fix this by providing the callback with:
-the status returned by the crypto engine (fd[status]) in case
it contains an error, OR
-a QI "No error" code otherwise; this will trigger the message:
platform caam_qi: 50000000: Queue Manager Interface: No error
which is fine, since QMan driver provides details about the cause of
failure

Cc: <[email protected]> # v4.12+
Fixes: 67c2315d ("crypto: caam - add Queue Interface (QI) backend support")
Signed-off-by: Horia Geantă <[email protected]>
---
drivers/crypto/caam/error.c | 1 +
drivers/crypto/caam/qi.c | 5 ++++-
drivers/crypto/caam/regs.h | 1 +
3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c
index 4f0d458..95da6ae 100644
--- a/drivers/crypto/caam/error.c
+++ b/drivers/crypto/caam/error.c
@@ -118,6 +118,7 @@ static const struct {
u8 value;
const char *error_text;
} qi_error_list[] = {
+ { 0x00, "No error" },
{ 0x1F, "Job terminated by FQ or ICID flush" },
{ 0x20, "FD format error"},
{ 0x21, "FD command format error"},
diff --git a/drivers/crypto/caam/qi.c b/drivers/crypto/caam/qi.c
index 0fe618e..19a378b 100644
--- a/drivers/crypto/caam/qi.c
+++ b/drivers/crypto/caam/qi.c
@@ -163,7 +163,10 @@ static void caam_fq_ern_cb(struct qman_portal *qm, struct qman_fq *fq,
dma_unmap_single(drv_req->drv_ctx->qidev, qm_fd_addr(fd),
sizeof(drv_req->fd_sgt), DMA_BIDIRECTIONAL);

- drv_req->cbk(drv_req, -EIO);
+ if (fd->status)
+ drv_req->cbk(drv_req, be32_to_cpu(fd->status));
+ else
+ drv_req->cbk(drv_req, JRSTA_SSRC_QI);
}

static struct qman_fq *create_caam_req_fq(struct device *qidev,
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 8591914..7c7ea8a 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -641,6 +641,7 @@ struct caam_job_ring {
#define JRSTA_SSRC_CCB_ERROR 0x20000000
#define JRSTA_SSRC_JUMP_HALT_USER 0x30000000
#define JRSTA_SSRC_DECO 0x40000000
+#define JRSTA_SSRC_QI 0x50000000
#define JRSTA_SSRC_JRERROR 0x60000000
#define JRSTA_SSRC_JUMP_HALT_CC 0x70000000

--
2.1.0

2019-07-18 14:48:13

by Iuliana Prodan

[permalink] [raw]
Subject: [PATCH 11/14] crypto: caam - free resources in case caam_rng registration failed

Check the return value of the hardware registration for caam_rng and free
resources in case of failure.

Fixes: 6e4e603a9 ("crypto: caam - Dynamic memory allocation for caam_rng_ctx object")
Signed-off-by: Iuliana Prodan <[email protected]>
---
drivers/crypto/caam/caamrng.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index 561bcb5..54c32d5 100644
--- a/drivers/crypto/caam/caamrng.c
+++ b/drivers/crypto/caam/caamrng.c
@@ -333,7 +333,10 @@ int caam_rng_init(struct device *ctrldev)
goto free_rng_ctx;

dev_info(dev, "registering rng-caam\n");
- return hwrng_register(&caam_rng);
+
+ err = hwrng_register(&caam_rng);
+ if (!err)
+ return err;

free_rng_ctx:
kfree(rng_ctx);
--
2.1.0

2019-07-18 15:00:00

by Horia Geanta

[permalink] [raw]
Subject: Re: [PATCH 03/14] crypto: caam - update IV only when crypto operation succeeds

On 7/18/2019 5:45 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).
>
The commit message is no longer in sync with the code base.

More exactly, after commit
334d37c9e263 ("crypto: caam - update IV using HW support")
there shouldn't be any panic, only a useless IV copy in case HW issued
an error.

> 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);
>