[PATCH 0/4] crypto: sun4i-ss: Fix problem reported by CONFIG_CRYPTO_EXTRA_TESTS

[PATCH 0/4] crypto: sun4i-ss: Fix problem reported by CONFIG_CRYPTO_EXTRA_TESTS

[Corentin Labbe]

Hello

Loading sun4i-ss with CONFIG_CRYPTO_EXTRA_TESTS, lead to hung tasks.
This patchset fix the two deadlock (one in hash, one in cipher) found.
This patchset fix also some invalid IV handling found while debugging
thoses issues.

Regards

Corentin Labbe (4):
  crypto: sun4i-ss: Handle better absence/presence of IV
  crypto: sun4i-ss: remove ivsize from ECB
  crypto: sun4i-ss: Fix invalid calculation of hash end
  crypto: sun4i-ss: fallback when length is not multiple of blocksize

 drivers/crypto/sunxi-ss/sun4i-ss-cipher.c | 67 ++++++++++++++++++-----
 drivers/crypto/sunxi-ss/sun4i-ss-core.c   | 19 ++++---
 drivers/crypto/sunxi-ss/sun4i-ss-hash.c   |  5 +-
 drivers/crypto/sunxi-ss/sun4i-ss.h        |  2 +
 4 files changed, 71 insertions(+), 22 deletions(-)

-- 
2.21.0

[PATCH 1/4] crypto: sun4i-ss: Handle better absence/presence of IV

[Corentin Labbe]

This patch remove the test against areq->info since sun4i-ss could work
without it (ECB).

Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/crypto/sunxi-ss/sun4i-ss-cipher.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
index 06df336488fb..4b60f8fdd1c8 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
@@ -41,11 +41,6 @@ static int sun4i_ss_opti_poll(struct skcipher_request *areq)
 	if (!areq->cryptlen)
 		return 0;
 
-	if (!areq->iv) {
-		dev_err_ratelimited(ss->dev, "ERROR: Empty IV\n");
-		return -EINVAL;
-	}
-
 	if (!areq->src || !areq->dst) {
 		dev_err_ratelimited(ss->dev, "ERROR: Some SGs are NULL\n");
 		return -EINVAL;
@@ -157,11 +152,6 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 	if (!areq->cryptlen)
 		return 0;
 
-	if (!areq->iv) {
-		dev_err_ratelimited(ss->dev, "ERROR: Empty IV\n");
-		return -EINVAL;
-	}
-
 	if (!areq->src || !areq->dst) {
 		dev_err_ratelimited(ss->dev, "ERROR: Some SGs are NULL\n");
 		return -EINVAL;
-- 
2.21.0

[PATCH 2/4] crypto: sun4i-ss: remove ivsize from ECB

[Corentin Labbe]

ECB algos does not need IV.

Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/crypto/sunxi-ss/sun4i-ss-core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
index 6b2e186dfa8b..3d9c5ad6cbb8 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
@@ -107,7 +107,6 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 		.decrypt        = sun4i_ss_ecb_aes_decrypt,
 		.min_keysize	= AES_MIN_KEY_SIZE,
 		.max_keysize	= AES_MAX_KEY_SIZE,
-		.ivsize		= AES_BLOCK_SIZE,
 		.base = {
 			.cra_name = "ecb(aes)",
 			.cra_driver_name = "ecb-aes-sun4i-ss",
@@ -190,7 +189,6 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 		.decrypt        = sun4i_ss_ecb_des3_decrypt,
 		.min_keysize    = DES3_EDE_KEY_SIZE,
 		.max_keysize    = DES3_EDE_KEY_SIZE,
-		.ivsize         = DES3_EDE_BLOCK_SIZE,
 		.base = {
 			.cra_name = "ecb(des3_ede)",
 			.cra_driver_name = "ecb-des3-sun4i-ss",
-- 
2.21.0

[PATCH 3/4] crypto: sun4i-ss: Fix invalid calculation of hash end

[Corentin Labbe]

When nbytes < 4, end is wronlgy set to a negative value which, due to
uint, is then interpreted to a large value leading to a deadlock in the
following code.

This patch fix this problem.

Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/crypto/sunxi-ss/sun4i-ss-hash.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-hash.c b/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
index a4b5ff2b72f8..f6936bb3b7be 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
@@ -240,7 +240,10 @@ static int sun4i_hash(struct ahash_request *areq)
 		}
 	} else {
 		/* Since we have the flag final, we can go up to modulo 4 */
-		end = ((areq->nbytes + op->len) / 4) * 4 - op->len;
+		if (areq->nbytes < 4)
+			end = 0;
+		else
+			end = ((areq->nbytes + op->len) / 4) * 4 - op->len;
 	}
 
 	/* TODO if SGlen % 4 and !op->len then DMA */
-- 
2.21.0

[PATCH 4/4] crypto: sun4i-ss: fallback when length is not multiple of blocksize

[Corentin Labbe]

sun4i-ss does not handle requests when length are not a multiple of
blocksize.
This patch adds a fallback for that case.

Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/crypto/sunxi-ss/sun4i-ss-cipher.c | 57 +++++++++++++++++++++--
 drivers/crypto/sunxi-ss/sun4i-ss-core.c   | 17 +++++--
 drivers/crypto/sunxi-ss/sun4i-ss.h        |  2 +
 3 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
index 4b60f8fdd1c8..b060a0810934 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
@@ -129,6 +129,8 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 	struct scatterlist *out_sg = areq->dst;
 	unsigned int ivsize = crypto_skcipher_ivsize(tfm);
 	struct sun4i_cipher_req_ctx *ctx = skcipher_request_ctx(areq);
+	struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
+	struct sun4i_ss_alg_template *algt;
 	u32 mode = ctx->mode;
 	/* when activating SS, the default FIFO space is SS_RX_DEFAULT(32) */
 	u32 rx_cnt = SS_RX_DEFAULT;
@@ -148,6 +150,7 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 	unsigned int obo = 0;	/* offset in bufo*/
 	unsigned int obl = 0;	/* length of data in bufo */
 	unsigned long flags;
+	bool need_fallback;
 
 	if (!areq->cryptlen)
 		return 0;
@@ -157,6 +160,10 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 		return -EINVAL;
 	}
 
+	algt = container_of(alg, struct sun4i_ss_alg_template, alg.crypto);
+	if (areq->cryptlen % algt->alg.crypto.base.cra_blocksize)
+		need_fallback = true;
+
 	/*
 	 * if we have only SGs with size multiple of 4,
 	 * we can use the SS optimized function
@@ -172,9 +179,24 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 		out_sg = sg_next(out_sg);
 	}
 
-	if (no_chunk == 1)
+	if (no_chunk == 1 && !need_fallback)
 		return sun4i_ss_opti_poll(areq);
 
+	if (need_fallback) {
+		SYNC_SKCIPHER_REQUEST_ON_STACK(subreq, op->fallback_tfm);
+		skcipher_request_set_sync_tfm(subreq, op->fallback_tfm);
+		skcipher_request_set_callback(subreq, areq->base.flags, NULL,
+					      NULL);
+		skcipher_request_set_crypt(subreq, areq->src, areq->dst,
+					   areq->cryptlen, areq->iv);
+		if (ctx->mode & SS_DECRYPTION)
+			err = crypto_skcipher_decrypt(subreq);
+		else
+			err = crypto_skcipher_encrypt(subreq);
+		skcipher_request_zero(subreq);
+		return err;
+	}
+
 	spin_lock_irqsave(&ss->slock, flags);
 
 	for (i = 0; i < op->keylen; i += 4)
@@ -448,6 +470,7 @@ int sun4i_ss_cipher_init(struct crypto_tfm *tfm)
 {
 	struct sun4i_tfm_ctx *op = crypto_tfm_ctx(tfm);
 	struct sun4i_ss_alg_template *algt;
+	const char *name = crypto_tfm_alg_name(tfm);
 
 	memset(op, 0, sizeof(struct sun4i_tfm_ctx));
 
@@ -458,9 +481,22 @@ int sun4i_ss_cipher_init(struct crypto_tfm *tfm)
 	crypto_skcipher_set_reqsize(__crypto_skcipher_cast(tfm),
 				    sizeof(struct sun4i_cipher_req_ctx));
 
+	op->fallback_tfm = crypto_alloc_sync_skcipher(name, 0, CRYPTO_ALG_NEED_FALLBACK);
+	if (IS_ERR(op->fallback_tfm)) {
+		dev_err(op->ss->dev, "ERROR: Cannot allocate fallback for %s %ld\n",
+			name, PTR_ERR(op->fallback_tfm));
+		return PTR_ERR(op->fallback_tfm);
+	}
+
 	return 0;
 }
 
+void sun4i_ss_cipher_exit(struct crypto_tfm *tfm)
+{
+	struct sun4i_tfm_ctx *op = crypto_tfm_ctx(tfm);
+	crypto_free_sync_skcipher(op->fallback_tfm);
+}
+
 /* check and set the AES key, prepare the mode to be used */
 int sun4i_ss_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
 			unsigned int keylen)
@@ -485,7 +521,11 @@ int sun4i_ss_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
 	}
 	op->keylen = keylen;
 	memcpy(op->key, key, keylen);
-	return 0;
+
+	crypto_sync_skcipher_clear_flags(op->fallback_tfm, CRYPTO_TFM_REQ_MASK);
+	crypto_sync_skcipher_set_flags(op->fallback_tfm, tfm->base.crt_flags & CRYPTO_TFM_REQ_MASK);
+
+	return crypto_sync_skcipher_setkey(op->fallback_tfm, key, keylen);
 }
 
 /* check and set the DES key, prepare the mode to be used */
@@ -515,7 +555,11 @@ int sun4i_ss_des_setkey(struct crypto_skcipher *tfm, const u8 *key,
 
 	op->keylen = keylen;
 	memcpy(op->key, key, keylen);
-	return 0;
+
+	crypto_sync_skcipher_clear_flags(op->fallback_tfm, CRYPTO_TFM_REQ_MASK);
+	crypto_sync_skcipher_set_flags(op->fallback_tfm, tfm->base.crt_flags & CRYPTO_TFM_REQ_MASK);
+
+	return crypto_sync_skcipher_setkey(op->fallback_tfm, key, keylen);
 }
 
 /* check and set the 3DES key, prepare the mode to be used */
@@ -531,5 +575,10 @@ int sun4i_ss_des3_setkey(struct crypto_skcipher *tfm, const u8 *key,
 
 	op->keylen = keylen;
 	memcpy(op->key, key, keylen);
-	return 0;
+
+	crypto_sync_skcipher_clear_flags(op->fallback_tfm, CRYPTO_TFM_REQ_MASK);
+	crypto_sync_skcipher_set_flags(op->fallback_tfm, tfm->base.crt_flags & CRYPTO_TFM_REQ_MASK);
+
+	return crypto_sync_skcipher_setkey(op->fallback_tfm, key, keylen);
+
 }
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
index 3d9c5ad6cbb8..a0ecfc8388a0 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
@@ -92,11 +92,12 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 			.cra_driver_name = "cbc-aes-sun4i-ss",
 			.cra_priority = 300,
 			.cra_blocksize = AES_BLOCK_SIZE,
-			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY,
+			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY | CRYPTO_ALG_NEED_FALLBACK,
 			.cra_ctxsize = sizeof(struct sun4i_tfm_ctx),
 			.cra_module = THIS_MODULE,
 			.cra_alignmask = 3,
 			.cra_init = sun4i_ss_cipher_init,
+			.cra_exit = sun4i_ss_cipher_exit,
 		}
 	}
 },
@@ -112,11 +113,12 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 			.cra_driver_name = "ecb-aes-sun4i-ss",
 			.cra_priority = 300,
 			.cra_blocksize = AES_BLOCK_SIZE,
-			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY,
+			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY | CRYPTO_ALG_NEED_FALLBACK,
 			.cra_ctxsize = sizeof(struct sun4i_tfm_ctx),
 			.cra_module = THIS_MODULE,
 			.cra_alignmask = 3,
 			.cra_init = sun4i_ss_cipher_init,
+			.cra_exit = sun4i_ss_cipher_exit,
 		}
 	}
 },
@@ -133,11 +135,12 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 			.cra_driver_name = "cbc-des-sun4i-ss",
 			.cra_priority = 300,
 			.cra_blocksize = DES_BLOCK_SIZE,
-			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY,
+			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY | CRYPTO_ALG_NEED_FALLBACK,
 			.cra_ctxsize = sizeof(struct sun4i_req_ctx),
 			.cra_module = THIS_MODULE,
 			.cra_alignmask = 3,
 			.cra_init = sun4i_ss_cipher_init,
+			.cra_exit = sun4i_ss_cipher_exit,
 		}
 	}
 },
@@ -153,11 +156,12 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 			.cra_driver_name = "ecb-des-sun4i-ss",
 			.cra_priority = 300,
 			.cra_blocksize = DES_BLOCK_SIZE,
-			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY,
+			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY | CRYPTO_ALG_NEED_FALLBACK,
 			.cra_ctxsize = sizeof(struct sun4i_req_ctx),
 			.cra_module = THIS_MODULE,
 			.cra_alignmask = 3,
 			.cra_init = sun4i_ss_cipher_init,
+			.cra_exit = sun4i_ss_cipher_exit,
 		}
 	}
 },
@@ -174,11 +178,12 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 			.cra_driver_name = "cbc-des3-sun4i-ss",
 			.cra_priority = 300,
 			.cra_blocksize = DES3_EDE_BLOCK_SIZE,
-			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY,
+			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY | CRYPTO_ALG_NEED_FALLBACK,
 			.cra_ctxsize = sizeof(struct sun4i_req_ctx),
 			.cra_module = THIS_MODULE,
 			.cra_alignmask = 3,
 			.cra_init = sun4i_ss_cipher_init,
+			.cra_exit = sun4i_ss_cipher_exit,
 		}
 	}
 },
@@ -194,10 +199,12 @@ static struct sun4i_ss_alg_template ss_algs[] = {
 			.cra_driver_name = "ecb-des3-sun4i-ss",
 			.cra_priority = 300,
 			.cra_blocksize = DES3_EDE_BLOCK_SIZE,
+			.cra_flags = CRYPTO_ALG_KERN_DRIVER_ONLY | CRYPTO_ALG_NEED_FALLBACK,
 			.cra_ctxsize = sizeof(struct sun4i_req_ctx),
 			.cra_module = THIS_MODULE,
 			.cra_alignmask = 3,
 			.cra_init = sun4i_ss_cipher_init,
+			.cra_exit = sun4i_ss_cipher_exit,
 		}
 	}
 },
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss.h b/drivers/crypto/sunxi-ss/sun4i-ss.h
index f3ac90692ac6..8c4ec9e93565 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss.h
+++ b/drivers/crypto/sunxi-ss/sun4i-ss.h
@@ -161,6 +161,7 @@ struct sun4i_tfm_ctx {
 	u32 keylen;
 	u32 keymode;
 	struct sun4i_ss_ctx *ss;
+	struct crypto_sync_skcipher *fallback_tfm;
 };
 
 struct sun4i_cipher_req_ctx {
@@ -203,6 +204,7 @@ int sun4i_ss_ecb_des3_encrypt(struct skcipher_request *areq);
 int sun4i_ss_ecb_des3_decrypt(struct skcipher_request *areq);
 
 int sun4i_ss_cipher_init(struct crypto_tfm *tfm);
+void sun4i_ss_cipher_exit(struct crypto_tfm *tfm);
 int sun4i_ss_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
 			unsigned int keylen);
 int sun4i_ss_des_setkey(struct crypto_skcipher *tfm, const u8 *key,
-- 
2.21.0

Re: [PATCH 0/4] crypto: sun4i-ss: Fix problem reported by CONFIG_CRYPTO_EXTRA_TESTS

[Herbert Xu]

On Thu, Apr 18, 2019 at 10:17:31AM +0200, Corentin Labbe wrote:
> Hello
> 
> Loading sun4i-ss with CONFIG_CRYPTO_EXTRA_TESTS, lead to hung tasks.
> This patchset fix the two deadlock (one in hash, one in cipher) found.
> This patchset fix also some invalid IV handling found while debugging
> thoses issues.
> 
> Regards
> 
> Corentin Labbe (4):
>   crypto: sun4i-ss: Handle better absence/presence of IV
>   crypto: sun4i-ss: remove ivsize from ECB
>   crypto: sun4i-ss: Fix invalid calculation of hash end
>   crypto: sun4i-ss: fallback when length is not multiple of blocksize
> 
>  drivers/crypto/sunxi-ss/sun4i-ss-cipher.c | 67 ++++++++++++++++++-----
>  drivers/crypto/sunxi-ss/sun4i-ss-core.c   | 19 ++++---
>  drivers/crypto/sunxi-ss/sun4i-ss-hash.c   |  5 +-
>  drivers/crypto/sunxi-ss/sun4i-ss.h        |  2 +
>  4 files changed, 71 insertions(+), 22 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