linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] crypto: fixes and cleanups
@ 2018-07-01  7:02 Gilad Ben-Yossef
  2018-07-01  7:02 ` [PATCH v2 1/6] crypto: ccree: fix finup Gilad Ben-Yossef
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Gilad Ben-Yossef @ 2018-07-01  7:02 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller
  Cc: Ofir Drang, Hadar Gat, linux-crypto, linux-kernel

The patch set fixes ccree IV handling, finup() operation (provided           
by Hadar Gat) and CTS-AES mode of operation along a code cleanup.                                             
                                                                                
Since our finup() method was obviously broken but was not detected              
by testmgr, I've added finup() test to testmgr to catch it next                 
time it happened.

Changes from v1:
- Add missing "static" qualifier reported by kbuild bot
- Add CTS-AES fix and some more cleanups


Gilad Ben-Yossef (5):
  crypto: testmgr: add hash finup tests
  crypto: ccree: fix iv handling
  crypto: ccree: remove dead legacy code
  crypto: ccree: use CBC-CS3 CTS mode
  crypto: ccree: rate limit debug print

Hadar Gat (1):
  crypto: ccree: fix finup

 crypto/testmgr.c                 |  55 +++++++++++++---
 drivers/crypto/ccree/cc_cipher.c | 137 +++++++++++++++++++++++++--------------
 drivers/crypto/ccree/cc_driver.c |   4 +-
 drivers/crypto/ccree/cc_hash.c   |  81 +++++------------------
 4 files changed, 153 insertions(+), 124 deletions(-)

-- 
2.7.4


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

* [PATCH v2 1/6] crypto: ccree: fix finup
  2018-07-01  7:02 [PATCH v2 0/6] crypto: fixes and cleanups Gilad Ben-Yossef
@ 2018-07-01  7:02 ` Gilad Ben-Yossef
  2018-07-01  7:02 ` [PATCH v2 2/6] crypto: testmgr: add hash finup tests Gilad Ben-Yossef
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Gilad Ben-Yossef @ 2018-07-01  7:02 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller
  Cc: Ofir Drang, Hadar Gat, stable, linux-crypto, linux-kernel

From: Hadar Gat <hadar.gat@arm.com>

finup() operation was incorrect, padding was missing.
Fix by setting the ccree HW to enable padding.

Signed-off-by: Hadar Gat <hadar.gat@arm.com>
[ gilad@benyossef.com: refactored for better code sharing ]
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Cc: stable@vger.kernel.org
---
 drivers/crypto/ccree/cc_hash.c | 81 +++++++++---------------------------------
 1 file changed, 16 insertions(+), 65 deletions(-)

diff --git a/drivers/crypto/ccree/cc_hash.c b/drivers/crypto/ccree/cc_hash.c
index 96ff777..e4ebde0 100644
--- a/drivers/crypto/ccree/cc_hash.c
+++ b/drivers/crypto/ccree/cc_hash.c
@@ -602,66 +602,7 @@ static int cc_hash_update(struct ahash_request *req)
 	return rc;
 }
 
-static int cc_hash_finup(struct ahash_request *req)
-{
-	struct ahash_req_ctx *state = ahash_request_ctx(req);
-	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
-	struct cc_hash_ctx *ctx = crypto_ahash_ctx(tfm);
-	u32 digestsize = crypto_ahash_digestsize(tfm);
-	struct scatterlist *src = req->src;
-	unsigned int nbytes = req->nbytes;
-	u8 *result = req->result;
-	struct device *dev = drvdata_to_dev(ctx->drvdata);
-	bool is_hmac = ctx->is_hmac;
-	struct cc_crypto_req cc_req = {};
-	struct cc_hw_desc desc[CC_MAX_HASH_SEQ_LEN];
-	unsigned int idx = 0;
-	int rc;
-	gfp_t flags = cc_gfp_flags(&req->base);
-
-	dev_dbg(dev, "===== %s-finup (%d) ====\n", is_hmac ? "hmac" : "hash",
-		nbytes);
-
-	if (cc_map_req(dev, state, ctx)) {
-		dev_err(dev, "map_ahash_source() failed\n");
-		return -EINVAL;
-	}
-
-	if (cc_map_hash_request_final(ctx->drvdata, state, src, nbytes, 1,
-				      flags)) {
-		dev_err(dev, "map_ahash_request_final() failed\n");
-		cc_unmap_req(dev, state, ctx);
-		return -ENOMEM;
-	}
-	if (cc_map_result(dev, state, digestsize)) {
-		dev_err(dev, "map_ahash_digest() failed\n");
-		cc_unmap_hash_request(dev, state, src, true);
-		cc_unmap_req(dev, state, ctx);
-		return -ENOMEM;
-	}
-
-	/* Setup request structure */
-	cc_req.user_cb = cc_hash_complete;
-	cc_req.user_arg = req;
-
-	idx = cc_restore_hash(desc, ctx, state, idx);
-
-	if (is_hmac)
-		idx = cc_fin_hmac(desc, req, idx);
-
-	idx = cc_fin_result(desc, req, idx);
-
-	rc = cc_send_request(ctx->drvdata, &cc_req, desc, idx, &req->base);
-	if (rc != -EINPROGRESS && rc != -EBUSY) {
-		dev_err(dev, "send_request() failed (rc=%d)\n", rc);
-		cc_unmap_hash_request(dev, state, src, true);
-		cc_unmap_result(dev, state, digestsize, result);
-		cc_unmap_req(dev, state, ctx);
-	}
-	return rc;
-}
-
-static int cc_hash_final(struct ahash_request *req)
+static int cc_do_finup(struct ahash_request *req, bool update)
 {
 	struct ahash_req_ctx *state = ahash_request_ctx(req);
 	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
@@ -678,21 +619,20 @@ static int cc_hash_final(struct ahash_request *req)
 	int rc;
 	gfp_t flags = cc_gfp_flags(&req->base);
 
-	dev_dbg(dev, "===== %s-final (%d) ====\n", is_hmac ? "hmac" : "hash",
-		nbytes);
+	dev_dbg(dev, "===== %s-%s (%d) ====\n", is_hmac ? "hmac" : "hash",
+		update ? "finup" : "final", nbytes);
 
 	if (cc_map_req(dev, state, ctx)) {
 		dev_err(dev, "map_ahash_source() failed\n");
 		return -EINVAL;
 	}
 
-	if (cc_map_hash_request_final(ctx->drvdata, state, src, nbytes, 0,
+	if (cc_map_hash_request_final(ctx->drvdata, state, src, nbytes, update,
 				      flags)) {
 		dev_err(dev, "map_ahash_request_final() failed\n");
 		cc_unmap_req(dev, state, ctx);
 		return -ENOMEM;
 	}
-
 	if (cc_map_result(dev, state, digestsize)) {
 		dev_err(dev, "map_ahash_digest() failed\n");
 		cc_unmap_hash_request(dev, state, src, true);
@@ -706,7 +646,7 @@ static int cc_hash_final(struct ahash_request *req)
 
 	idx = cc_restore_hash(desc, ctx, state, idx);
 
-	/* "DO-PAD" must be enabled only when writing current length to HW */
+	/* Pad the hash */
 	hw_desc_init(&desc[idx]);
 	set_cipher_do(&desc[idx], DO_PAD);
 	set_cipher_mode(&desc[idx], ctx->hw_mode);
@@ -731,6 +671,17 @@ static int cc_hash_final(struct ahash_request *req)
 	return rc;
 }
 
+static int cc_hash_finup(struct ahash_request *req)
+{
+	return cc_do_finup(req, true);
+}
+
+
+static int cc_hash_final(struct ahash_request *req)
+{
+	return cc_do_finup(req, false);
+}
+
 static int cc_hash_init(struct ahash_request *req)
 {
 	struct ahash_req_ctx *state = ahash_request_ctx(req);
-- 
2.7.4


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

* [PATCH v2 2/6] crypto: testmgr: add hash finup tests
  2018-07-01  7:02 [PATCH v2 0/6] crypto: fixes and cleanups Gilad Ben-Yossef
  2018-07-01  7:02 ` [PATCH v2 1/6] crypto: ccree: fix finup Gilad Ben-Yossef
@ 2018-07-01  7:02 ` Gilad Ben-Yossef
  2018-07-01  7:02 ` [PATCH v2 3/6] crypto: ccree: fix iv handling Gilad Ben-Yossef
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Gilad Ben-Yossef @ 2018-07-01  7:02 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller
  Cc: Ofir Drang, Hadar Gat, linux-crypto, linux-kernel

The testmgr hash tests were testing init, digest, update and final
methods but not the finup method. Add a test for this one too.

While doing this, make sure we only run the partial tests once with
the digest tests and skip them with the final and finup tests since
they are the same.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 crypto/testmgr.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 11e4535..ca475f6 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -259,9 +259,15 @@ static int ahash_partial_update(struct ahash_request **preq,
 	return ret;
 }
 
+enum hash_test {
+	HASH_TEST_DIGEST,
+	HASH_TEST_FINAL,
+	HASH_TEST_FINUP
+};
+
 static int __test_hash(struct crypto_ahash *tfm,
 		       const struct hash_testvec *template, unsigned int tcount,
-		       bool use_digest, const int align_offset)
+		       enum hash_test test_type, const int align_offset)
 {
 	const char *algo = crypto_tfm_alg_driver_name(crypto_ahash_tfm(tfm));
 	size_t digest_size = crypto_ahash_digestsize(tfm);
@@ -332,14 +338,17 @@ static int __test_hash(struct crypto_ahash *tfm,
 		}
 
 		ahash_request_set_crypt(req, sg, result, template[i].psize);
-		if (use_digest) {
+		switch (test_type) {
+		case HASH_TEST_DIGEST:
 			ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
 			if (ret) {
 				pr_err("alg: hash: digest failed on test %d "
 				       "for %s: ret=%d\n", j, algo, -ret);
 				goto out;
 			}
-		} else {
+			break;
+
+		case HASH_TEST_FINAL:
 			memset(result, 1, digest_size);
 			ret = crypto_wait_req(crypto_ahash_init(req), &wait);
 			if (ret) {
@@ -371,6 +380,29 @@ static int __test_hash(struct crypto_ahash *tfm,
 				       "for %s: ret=%d\n", j, algo, -ret);
 				goto out;
 			}
+			break;
+
+		case HASH_TEST_FINUP:
+			memset(result, 1, digest_size);
+			ret = crypto_wait_req(crypto_ahash_init(req), &wait);
+			if (ret) {
+				pr_err("alg: hash: init failed on test %d "
+				       "for %s: ret=%d\n", j, algo, -ret);
+				goto out;
+			}
+			ret = ahash_guard_result(result, 1, digest_size);
+			if (ret) {
+				pr_err("alg: hash: init failed on test %d "
+				       "for %s: used req->result\n", j, algo);
+				goto out;
+			}
+			ret = crypto_wait_req(crypto_ahash_finup(req), &wait);
+			if (ret) {
+				pr_err("alg: hash: final failed on test %d "
+				       "for %s: ret=%d\n", j, algo, -ret);
+				goto out;
+			}
+			break;
 		}
 
 		if (memcmp(result, template[i].digest,
@@ -383,6 +415,9 @@ static int __test_hash(struct crypto_ahash *tfm,
 		}
 	}
 
+	if (test_type)
+		goto out;
+
 	j = 0;
 	for (i = 0; i < tcount; i++) {
 		/* alignment tests are only done with continuous buffers */
@@ -540,24 +575,24 @@ static int __test_hash(struct crypto_ahash *tfm,
 
 static int test_hash(struct crypto_ahash *tfm,
 		     const struct hash_testvec *template,
-		     unsigned int tcount, bool use_digest)
+		     unsigned int tcount, enum hash_test test_type)
 {
 	unsigned int alignmask;
 	int ret;
 
-	ret = __test_hash(tfm, template, tcount, use_digest, 0);
+	ret = __test_hash(tfm, template, tcount, test_type, 0);
 	if (ret)
 		return ret;
 
 	/* test unaligned buffers, check with one byte offset */
-	ret = __test_hash(tfm, template, tcount, use_digest, 1);
+	ret = __test_hash(tfm, template, tcount, test_type, 1);
 	if (ret)
 		return ret;
 
 	alignmask = crypto_tfm_alg_alignmask(&tfm->base);
 	if (alignmask) {
 		/* Check if alignment mask for tfm is correctly set. */
-		ret = __test_hash(tfm, template, tcount, use_digest,
+		ret = __test_hash(tfm, template, tcount, test_type,
 				  alignmask + 1);
 		if (ret)
 			return ret;
@@ -1803,9 +1838,11 @@ static int __alg_test_hash(const struct hash_testvec *template,
 		return PTR_ERR(tfm);
 	}
 
-	err = test_hash(tfm, template, tcount, true);
+	err = test_hash(tfm, template, tcount, HASH_TEST_DIGEST);
+	if (!err)
+		err = test_hash(tfm, template, tcount, HASH_TEST_FINAL);
 	if (!err)
-		err = test_hash(tfm, template, tcount, false);
+		err = test_hash(tfm, template, tcount, HASH_TEST_FINUP);
 	crypto_free_ahash(tfm);
 	return err;
 }
-- 
2.7.4


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

* [PATCH v2 3/6] crypto: ccree: fix iv handling
  2018-07-01  7:02 [PATCH v2 0/6] crypto: fixes and cleanups Gilad Ben-Yossef
  2018-07-01  7:02 ` [PATCH v2 1/6] crypto: ccree: fix finup Gilad Ben-Yossef
  2018-07-01  7:02 ` [PATCH v2 2/6] crypto: testmgr: add hash finup tests Gilad Ben-Yossef
@ 2018-07-01  7:02 ` Gilad Ben-Yossef
  2018-07-01  7:02 ` [PATCH v2 4/6] crypto: ccree: remove dead legacy code Gilad Ben-Yossef
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Gilad Ben-Yossef @ 2018-07-01  7:02 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller
  Cc: Ofir Drang, Hadar Gat, stable, linux-crypto, linux-kernel

We were copying our last cipher block into the request for use as IV for
all modes of operations. Fix this by discerning the behaviour based on
the mode of operation used: copy ciphertext for CBC, update counter for
CTR.

CC: stable@vger.kernel.org
Fixes: 63ee04c8b491 ("crypto: ccree - add skcipher support")
Reported by: Hadar Gat <hadar.gat@arm.com>
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/crypto/ccree/cc_cipher.c | 111 +++++++++++++++++++++++++++++----------
 1 file changed, 84 insertions(+), 27 deletions(-)

diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index d2810c1..958ced3 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -593,34 +593,82 @@ static void cc_setup_cipher_data(struct crypto_tfm *tfm,
 	}
 }
 
+/*
+ * Update a CTR-AES 128 bit counter
+ */
+static void cc_update_ctr(u8 *ctr, unsigned int increment)
+{
+	if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) ||
+	    IS_ALIGNED((unsigned long)ctr, 8)) {
+
+		__be64 *high_be = (__be64 *)ctr;
+		__be64 *low_be = high_be + 1;
+		u64 orig_low = __be64_to_cpu(*low_be);
+		u64 new_low = orig_low + (u64)increment;
+
+		*low_be = __cpu_to_be64(new_low);
+
+		if (new_low < orig_low)
+			*high_be = __cpu_to_be64(__be64_to_cpu(*high_be) + 1);
+	} else {
+		u8 *pos = (ctr + AES_BLOCK_SIZE);
+		u8 val;
+		unsigned int size;
+
+		for (; increment; increment--)
+			for (size = AES_BLOCK_SIZE; size; size--) {
+				val = *--pos + 1;
+				*pos = val;
+				if (val)
+					break;
+			}
+	}
+}
+
 static void cc_cipher_complete(struct device *dev, void *cc_req, int err)
 {
 	struct skcipher_request *req = (struct skcipher_request *)cc_req;
 	struct scatterlist *dst = req->dst;
 	struct scatterlist *src = req->src;
 	struct cipher_req_ctx *req_ctx = skcipher_request_ctx(req);
-	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
-	unsigned int ivsize = crypto_skcipher_ivsize(tfm);
+	struct crypto_skcipher *sk_tfm = crypto_skcipher_reqtfm(req);
+	struct crypto_tfm *tfm = crypto_skcipher_tfm(sk_tfm);
+	struct cc_cipher_ctx *ctx_p = crypto_tfm_ctx(tfm);
+	unsigned int ivsize = crypto_skcipher_ivsize(sk_tfm);
+	unsigned int len;
 
-	cc_unmap_cipher_request(dev, req_ctx, ivsize, src, dst);
-	kzfree(req_ctx->iv);
+	switch (ctx_p->cipher_mode) {
+	case DRV_CIPHER_CBC:
+		/*
+		 * The crypto API expects us to set the req->iv to the last
+		 * ciphertext block. For encrypt, simply copy from the result.
+		 * For decrypt, we must copy from a saved buffer since this
+		 * could be an in-place decryption operation and the src is
+		 * lost by this point.
+		 */
+		if (req_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT)  {
+			memcpy(req->iv, req_ctx->backup_info, ivsize);
+			kzfree(req_ctx->backup_info);
+		} else if (!err) {
+			len = req->cryptlen - ivsize;
+			scatterwalk_map_and_copy(req->iv, req->dst, len,
+						 ivsize, 0);
+		}
+		break;
 
-	/*
-	 * The crypto API expects us to set the req->iv to the last
-	 * ciphertext block. For encrypt, simply copy from the result.
-	 * For decrypt, we must copy from a saved buffer since this
-	 * could be an in-place decryption operation and the src is
-	 * lost by this point.
-	 */
-	if (req_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT)  {
-		memcpy(req->iv, req_ctx->backup_info, ivsize);
-		kzfree(req_ctx->backup_info);
-	} else if (!err) {
-		scatterwalk_map_and_copy(req->iv, req->dst,
-					 (req->cryptlen - ivsize),
-					 ivsize, 0);
+	case DRV_CIPHER_CTR:
+		/* Compute the counter of the last block */
+		len = ALIGN(req->cryptlen, AES_BLOCK_SIZE) / AES_BLOCK_SIZE;
+		cc_update_ctr((u8 *)req->iv, len);
+		break;
+
+	default:
+		break;
 	}
 
+	cc_unmap_cipher_request(dev, req_ctx, ivsize, src, dst);
+	kzfree(req_ctx->iv);
+
 	skcipher_request_complete(req, err);
 }
 
@@ -752,20 +800,29 @@ static int cc_cipher_encrypt(struct skcipher_request *req)
 static int cc_cipher_decrypt(struct skcipher_request *req)
 {
 	struct crypto_skcipher *sk_tfm = crypto_skcipher_reqtfm(req);
+	struct crypto_tfm *tfm = crypto_skcipher_tfm(sk_tfm);
+	struct cc_cipher_ctx *ctx_p = crypto_tfm_ctx(tfm);
 	struct cipher_req_ctx *req_ctx = skcipher_request_ctx(req);
 	unsigned int ivsize = crypto_skcipher_ivsize(sk_tfm);
 	gfp_t flags = cc_gfp_flags(&req->base);
+	unsigned int len;
 
-	/*
-	 * Allocate and save the last IV sized bytes of the source, which will
-	 * be lost in case of in-place decryption and might be needed for CTS.
-	 */
-	req_ctx->backup_info = kmalloc(ivsize, flags);
-	if (!req_ctx->backup_info)
-		return -ENOMEM;
+	if (ctx_p->cipher_mode == DRV_CIPHER_CBC) {
+
+		/* Allocate and save the last IV sized bytes of the source,
+		 * which will be lost in case of in-place decryption.
+		 */
+		req_ctx->backup_info = kzalloc(ivsize, flags);
+		if (!req_ctx->backup_info)
+			return -ENOMEM;
+
+		len = req->cryptlen - ivsize;
+		scatterwalk_map_and_copy(req_ctx->backup_info, req->src, len,
+					 ivsize, 0);
+	} else {
+		req_ctx->backup_info = NULL;
+	}
 
-	scatterwalk_map_and_copy(req_ctx->backup_info, req->src,
-				 (req->cryptlen - ivsize), ivsize, 0);
 	req_ctx->is_giv = false;
 
 	return cc_cipher_process(req, DRV_CRYPTO_DIRECTION_DECRYPT);
-- 
2.7.4


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

* [PATCH v2 4/6] crypto: ccree: remove dead legacy code
  2018-07-01  7:02 [PATCH v2 0/6] crypto: fixes and cleanups Gilad Ben-Yossef
                   ` (2 preceding siblings ...)
  2018-07-01  7:02 ` [PATCH v2 3/6] crypto: ccree: fix iv handling Gilad Ben-Yossef
@ 2018-07-01  7:02 ` Gilad Ben-Yossef
  2018-07-01  7:02 ` [PATCH v2 5/6] crypto: ccree: use CBC-CS3 CTS mode Gilad Ben-Yossef
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Gilad Ben-Yossef @ 2018-07-01  7:02 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller
  Cc: Ofir Drang, Hadar Gat, linux-crypto, linux-kernel

Remove legacy code no longer used by anything.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/crypto/ccree/cc_cipher.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 958ced3..5d12372 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -730,12 +730,6 @@ static int cc_cipher_process(struct skcipher_request *req,
 	cc_req.user_cb = (void *)cc_cipher_complete;
 	cc_req.user_arg = (void *)req;
 
-#ifdef ENABLE_CYCLE_COUNT
-	cc_req.op_type = (direction == DRV_CRYPTO_DIRECTION_DECRYPT) ?
-		STAT_OP_TYPE_DECODE : STAT_OP_TYPE_ENCODE;
-
-#endif
-
 	/* Setup request context */
 	req_ctx->gen_ctx.op_type = direction;
 
-- 
2.7.4


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

* [PATCH v2 5/6] crypto: ccree: use CBC-CS3 CTS mode
  2018-07-01  7:02 [PATCH v2 0/6] crypto: fixes and cleanups Gilad Ben-Yossef
                   ` (3 preceding siblings ...)
  2018-07-01  7:02 ` [PATCH v2 4/6] crypto: ccree: remove dead legacy code Gilad Ben-Yossef
@ 2018-07-01  7:02 ` Gilad Ben-Yossef
  2018-07-01  7:02 ` [PATCH v2 6/6] crypto: ccree: rate limit debug print Gilad Ben-Yossef
  2018-07-08 16:46 ` [PATCH v2 0/6] crypto: fixes and cleanups Herbert Xu
  6 siblings, 0 replies; 9+ messages in thread
From: Gilad Ben-Yossef @ 2018-07-01  7:02 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller
  Cc: Ofir Drang, Hadar Gat, linux-crypto, linux-kernel

The ccree driver implemented NIST 800-38A CBC-CS2 ciphertext format,
which only reverses the last two blocks if the stolen ciphertext amount
are none zero. Move it to the kernel chosen format of CBC-CS3  which swaps
the final blocks unconditionally and rename it to "cts" now that it
complies with the kernel format and passes the self tests.

Ironically, the CryptoCell REE HW does just that, so the fix is dropping
the code that forced it to use plain CBC if the ciphertext was block
aligned.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/crypto/ccree/cc_cipher.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 5d12372..7a80963 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -687,7 +687,7 @@ static int cc_cipher_process(struct skcipher_request *req,
 	struct device *dev = drvdata_to_dev(ctx_p->drvdata);
 	struct cc_hw_desc desc[MAX_ABLKCIPHER_SEQ_LEN];
 	struct cc_crypto_req cc_req = {};
-	int rc, cts_restore_flag = 0;
+	int rc;
 	unsigned int seq_len = 0;
 	gfp_t flags = cc_gfp_flags(&req->base);
 
@@ -719,13 +719,6 @@ static int cc_cipher_process(struct skcipher_request *req,
 		goto exit_process;
 	}
 
-	/*For CTS in case of data size aligned to 16 use CBC mode*/
-	if (((nbytes % AES_BLOCK_SIZE) == 0) &&
-	    ctx_p->cipher_mode == DRV_CIPHER_CBC_CTS) {
-		ctx_p->cipher_mode = DRV_CIPHER_CBC;
-		cts_restore_flag = 1;
-	}
-
 	/* Setup request structure */
 	cc_req.user_cb = (void *)cc_cipher_complete;
 	cc_req.user_arg = (void *)req;
@@ -770,9 +763,6 @@ static int cc_cipher_process(struct skcipher_request *req,
 	}
 
 exit_process:
-	if (cts_restore_flag)
-		ctx_p->cipher_mode = DRV_CIPHER_CBC_CTS;
-
 	if (rc != -EINPROGRESS && rc != -EBUSY) {
 		kzfree(req_ctx->backup_info);
 		kzfree(req_ctx->iv);
@@ -1026,8 +1016,8 @@ static const struct cc_alg_template skcipher_algs[] = {
 		.min_hw_rev = CC_HW_REV_712,
 	},
 	{
-		.name = "cts1(cbc(paes))",
-		.driver_name = "cts1-cbc-paes-ccree",
+		.name = "cts(cbc(paes))",
+		.driver_name = "cts-cbc-paes-ccree",
 		.blocksize = AES_BLOCK_SIZE,
 		.type = CRYPTO_ALG_TYPE_ABLKCIPHER,
 		.template_skcipher = {
@@ -1261,8 +1251,8 @@ static const struct cc_alg_template skcipher_algs[] = {
 		.min_hw_rev = CC_HW_REV_630,
 	},
 	{
-		.name = "cts1(cbc(aes))",
-		.driver_name = "cts1-cbc-aes-ccree",
+		.name = "cts(cbc(aes))",
+		.driver_name = "cts-cbc-aes-ccree",
 		.blocksize = AES_BLOCK_SIZE,
 		.type = CRYPTO_ALG_TYPE_ABLKCIPHER,
 		.template_skcipher = {
-- 
2.7.4


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

* [PATCH v2 6/6] crypto: ccree: rate limit debug print
  2018-07-01  7:02 [PATCH v2 0/6] crypto: fixes and cleanups Gilad Ben-Yossef
                   ` (4 preceding siblings ...)
  2018-07-01  7:02 ` [PATCH v2 5/6] crypto: ccree: use CBC-CS3 CTS mode Gilad Ben-Yossef
@ 2018-07-01  7:02 ` Gilad Ben-Yossef
  2018-07-02 12:58   ` Geert Uytterhoeven
  2018-07-08 16:46 ` [PATCH v2 0/6] crypto: fixes and cleanups Herbert Xu
  6 siblings, 1 reply; 9+ messages in thread
From: Gilad Ben-Yossef @ 2018-07-01  7:02 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller
  Cc: Ofir Drang, Hadar Gat, linux-crypto, linux-kernel

A debug print about register status post interrupt can happen
quite often. Rate limit it to avoid cluttering the log.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/crypto/ccree/cc_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index bd974fe..1ff229c 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -131,8 +131,8 @@ static irqreturn_t cc_isr(int irq, void *dev_id)
 	}
 
 	if (irr) {
-		dev_dbg(dev, "IRR includes unknown cause bits (0x%08X)\n",
-			irr);
+		dev_dbg_ratelimited(dev, "IRR includes unknown cause bits (0x%08X)\n",
+				    irr);
 		/* Just warning */
 	}
 
-- 
2.7.4


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

* Re: [PATCH v2 6/6] crypto: ccree: rate limit debug print
  2018-07-01  7:02 ` [PATCH v2 6/6] crypto: ccree: rate limit debug print Gilad Ben-Yossef
@ 2018-07-02 12:58   ` Geert Uytterhoeven
  0 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2018-07-02 12:58 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: Herbert Xu, David S. Miller, Ofir Drang, hadar.gat,
	Linux Crypto Mailing List, Linux Kernel Mailing List

Hi Gilad,

On Sun, Jul 1, 2018 at 9:05 AM Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> A debug print about register status post interrupt can happen
> quite often. Rate limit it to avoid cluttering the log.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>

Thanks for your patch!

> --- a/drivers/crypto/ccree/cc_driver.c
> +++ b/drivers/crypto/ccree/cc_driver.c
> @@ -131,8 +131,8 @@ static irqreturn_t cc_isr(int irq, void *dev_id)
>         }
>
>         if (irr) {
> -               dev_dbg(dev, "IRR includes unknown cause bits (0x%08X)\n",
> -                       irr);
> +               dev_dbg_ratelimited(dev, "IRR includes unknown cause bits (0x%08X)\n",
> +                                   irr);
>                 /* Just warning */
>         }

    cc_isr: 932 callbacks suppressed

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Note that it still printed 1410 lines like:

    ccree e6601000.crypto: Got IRR=0x008000D8 (actual value may differ)

but perhaps you do intend to see these during debugging?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH v2 0/6] crypto: fixes and cleanups
  2018-07-01  7:02 [PATCH v2 0/6] crypto: fixes and cleanups Gilad Ben-Yossef
                   ` (5 preceding siblings ...)
  2018-07-01  7:02 ` [PATCH v2 6/6] crypto: ccree: rate limit debug print Gilad Ben-Yossef
@ 2018-07-08 16:46 ` Herbert Xu
  6 siblings, 0 replies; 9+ messages in thread
From: Herbert Xu @ 2018-07-08 16:46 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: David S. Miller, Ofir Drang, Hadar Gat, linux-crypto, linux-kernel

On Sun, Jul 01, 2018 at 08:02:33AM +0100, Gilad Ben-Yossef wrote:
> The patch set fixes ccree IV handling, finup() operation (provided           
> by Hadar Gat) and CTS-AES mode of operation along a code cleanup.                                             
>                                                                                 
> Since our finup() method was obviously broken but was not detected              
> by testmgr, I've added finup() test to testmgr to catch it next                 
> time it happened.
> 
> Changes from v1:
> - Add missing "static" qualifier reported by kbuild bot
> - Add CTS-AES fix and some more cleanups
> 
> 
> Gilad Ben-Yossef (5):
>   crypto: testmgr: add hash finup tests
>   crypto: ccree: fix iv handling
>   crypto: ccree: remove dead legacy code
>   crypto: ccree: use CBC-CS3 CTS mode
>   crypto: ccree: rate limit debug print
> 
> Hadar Gat (1):
>   crypto: ccree: fix finup
> 
>  crypto/testmgr.c                 |  55 +++++++++++++---
>  drivers/crypto/ccree/cc_cipher.c | 137 +++++++++++++++++++++++++--------------
>  drivers/crypto/ccree/cc_driver.c |   4 +-
>  drivers/crypto/ccree/cc_hash.c   |  81 +++++------------------
>  4 files changed, 153 insertions(+), 124 deletions(-)

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

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

end of thread, other threads:[~2018-07-08 16:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-01  7:02 [PATCH v2 0/6] crypto: fixes and cleanups Gilad Ben-Yossef
2018-07-01  7:02 ` [PATCH v2 1/6] crypto: ccree: fix finup Gilad Ben-Yossef
2018-07-01  7:02 ` [PATCH v2 2/6] crypto: testmgr: add hash finup tests Gilad Ben-Yossef
2018-07-01  7:02 ` [PATCH v2 3/6] crypto: ccree: fix iv handling Gilad Ben-Yossef
2018-07-01  7:02 ` [PATCH v2 4/6] crypto: ccree: remove dead legacy code Gilad Ben-Yossef
2018-07-01  7:02 ` [PATCH v2 5/6] crypto: ccree: use CBC-CS3 CTS mode Gilad Ben-Yossef
2018-07-01  7:02 ` [PATCH v2 6/6] crypto: ccree: rate limit debug print Gilad Ben-Yossef
2018-07-02 12:58   ` Geert Uytterhoeven
2018-07-08 16:46 ` [PATCH v2 0/6] crypto: fixes and cleanups Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).