All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs
@ 2019-01-23 11:59 Lars Persson
  2019-01-23 11:59 ` [PATCH 1/7] crypto: axis - remove sha384 support for artpec7 Lars Persson
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Lars Persson @ 2019-01-23 11:59 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: Lars Persson

Hi

This series brings to mainline fixes done during our product development and
fixes for errors detected by the IPsec testsuite in LTP.

Lars Persson (6):
  crypto: axis - remove sha384 support for artpec7
  crypto: axis - remove sha512 support for artpec7
  crypto: axis - fix for recursive locking from bottom half
  crypto: axis - give DMA the start of the status buffer
  crypto: axis - support variable AEAD tag length
  crypto: axis - use a constant time tag compare

Vincent Whitchurch (1):
  crypto: axis - move request unmap outside of the queue lock

 drivers/crypto/axis/artpec6_crypto.c | 317 ++++++++---------------------------
 1 file changed, 71 insertions(+), 246 deletions(-)

-- 
2.11.0


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

* [PATCH 1/7] crypto: axis - remove sha384 support for artpec7
  2019-01-23 11:59 [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Lars Persson
@ 2019-01-23 11:59 ` Lars Persson
  2019-01-23 11:59 ` [PATCH 2/7] crypto: axis - remove sha512 " Lars Persson
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Lars Persson @ 2019-01-23 11:59 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: Lars Persson

The hardware implementation of SHA384 was not correct and it cannot
be used in any situation.

Signed-off-by: Lars Persson <larper@axis.com>
---
 drivers/crypto/axis/artpec6_crypto.c | 107 +----------------------------------
 1 file changed, 2 insertions(+), 105 deletions(-)

diff --git a/drivers/crypto/axis/artpec6_crypto.c b/drivers/crypto/axis/artpec6_crypto.c
index f3442c2bdbdc..e4cbb2d11514 100644
--- a/drivers/crypto/axis/artpec6_crypto.c
+++ b/drivers/crypto/axis/artpec6_crypto.c
@@ -135,7 +135,6 @@
 #define regk_crypto_ext         0x00000001
 #define regk_crypto_hmac_sha1   0x00000007
 #define regk_crypto_hmac_sha256 0x00000009
-#define regk_crypto_hmac_sha384 0x0000000b
 #define regk_crypto_hmac_sha512 0x0000000d
 #define regk_crypto_init        0x00000000
 #define regk_crypto_key_128     0x00000000
@@ -144,7 +143,6 @@
 #define regk_crypto_null        0x00000000
 #define regk_crypto_sha1        0x00000006
 #define regk_crypto_sha256      0x00000008
-#define regk_crypto_sha384      0x0000000a
 #define regk_crypto_sha512      0x0000000c
 
 /* DMA descriptor structures */
@@ -190,7 +188,6 @@ struct pdma_stat_descr {
 /* Hash modes (including HMAC variants) */
 #define ARTPEC6_CRYPTO_HASH_SHA1	1
 #define ARTPEC6_CRYPTO_HASH_SHA256	2
-#define ARTPEC6_CRYPTO_HASH_SHA384	3
 #define ARTPEC6_CRYPTO_HASH_SHA512	4
 
 /* Crypto modes */
@@ -1315,8 +1312,7 @@ static int artpec6_crypto_prepare_hash(struct ahash_request *areq)
 	struct artpec6_hashalg_context *ctx = crypto_tfm_ctx(areq->base.tfm);
 	struct artpec6_hash_request_context *req_ctx = ahash_request_ctx(areq);
 	size_t digestsize = crypto_ahash_digestsize(crypto_ahash_reqtfm(areq));
-	size_t contextsize = digestsize == SHA384_DIGEST_SIZE ?
-		SHA512_DIGEST_SIZE : digestsize;
+	size_t contextsize = digestsize;
 	size_t blocksize = crypto_tfm_alg_blocksize(
 		crypto_ahash_tfm(crypto_ahash_reqtfm(areq)));
 	struct artpec6_crypto_req_common *common = &req_ctx->common;
@@ -1456,7 +1452,6 @@ static int artpec6_crypto_prepare_hash(struct ahash_request *areq)
 
 	/* Finalize */
 	if (req_ctx->hash_flags & HASH_FLAG_FINALIZE) {
-		bool needtrim = contextsize != digestsize;
 		size_t hash_pad_len;
 		u64 digest_bits;
 		u32 oper;
@@ -1502,19 +1497,10 @@ static int artpec6_crypto_prepare_hash(struct ahash_request *areq)
 		/* Descriptor for the final result */
 		error = artpec6_crypto_setup_in_descr(common, areq->result,
 						      digestsize,
-						      !needtrim);
+						      true);
 		if (error)
 			return error;
 
-		if (needtrim) {
-			/* Discard the extra context bytes for SHA-384 */
-			error = artpec6_crypto_setup_in_descr(common,
-					req_ctx->partial_buffer,
-					digestsize - contextsize, true);
-			if (error)
-				return error;
-		}
-
 	} else { /* This is not the final operation for this request */
 		if (!run_hw)
 			return ARTPEC6_CRYPTO_PREPARE_HASH_NO_START;
@@ -2266,9 +2252,6 @@ artpec6_crypto_init_hash(struct ahash_request *req, u8 type, int hmac)
 	case ARTPEC6_CRYPTO_HASH_SHA256:
 		oper = hmac ? regk_crypto_hmac_sha256 : regk_crypto_sha256;
 		break;
-	case ARTPEC6_CRYPTO_HASH_SHA384:
-		oper = hmac ? regk_crypto_hmac_sha384 : regk_crypto_sha384;
-		break;
 	case ARTPEC6_CRYPTO_HASH_SHA512:
 		oper = hmac ? regk_crypto_hmac_sha512 : regk_crypto_sha512;
 		break;
@@ -2368,22 +2351,6 @@ static int artpec6_crypto_sha256_digest(struct ahash_request *req)
 	return artpec6_crypto_prepare_submit_hash(req);
 }
 
-static int __maybe_unused artpec6_crypto_sha384_init(struct ahash_request *req)
-{
-	return artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA384, 0);
-}
-
-static int __maybe_unused
-artpec6_crypto_sha384_digest(struct ahash_request *req)
-{
-	struct artpec6_hash_request_context *req_ctx = ahash_request_ctx(req);
-
-	artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA384, 0);
-	req_ctx->hash_flags |= HASH_FLAG_UPDATE | HASH_FLAG_FINALIZE;
-
-	return artpec6_crypto_prepare_submit_hash(req);
-}
-
 static int artpec6_crypto_sha512_init(struct ahash_request *req)
 {
 	return artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA512, 0);
@@ -2404,12 +2371,6 @@ static int artpec6_crypto_hmac_sha256_init(struct ahash_request *req)
 	return artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA256, 1);
 }
 
-static int __maybe_unused
-artpec6_crypto_hmac_sha384_init(struct ahash_request *req)
-{
-	return artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA384, 1);
-}
-
 static int artpec6_crypto_hmac_sha512_init(struct ahash_request *req)
 {
 	return artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA512, 1);
@@ -2425,17 +2386,6 @@ static int artpec6_crypto_hmac_sha256_digest(struct ahash_request *req)
 	return artpec6_crypto_prepare_submit_hash(req);
 }
 
-static int __maybe_unused
-artpec6_crypto_hmac_sha384_digest(struct ahash_request *req)
-{
-	struct artpec6_hash_request_context *req_ctx = ahash_request_ctx(req);
-
-	artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA384, 1);
-	req_ctx->hash_flags |= HASH_FLAG_UPDATE | HASH_FLAG_FINALIZE;
-
-	return artpec6_crypto_prepare_submit_hash(req);
-}
-
 static int artpec6_crypto_hmac_sha512_digest(struct ahash_request *req)
 {
 	struct artpec6_hash_request_context *req_ctx = ahash_request_ctx(req);
@@ -2480,12 +2430,6 @@ static int artpec6_crypto_ahash_init_hmac_sha256(struct crypto_tfm *tfm)
 	return artpec6_crypto_ahash_init_common(tfm, "sha256");
 }
 
-static int __maybe_unused
-artpec6_crypto_ahash_init_hmac_sha384(struct crypto_tfm *tfm)
-{
-	return artpec6_crypto_ahash_init_common(tfm, "sha384");
-}
-
 static int artpec6_crypto_ahash_init_hmac_sha512(struct crypto_tfm *tfm)
 {
 	return artpec6_crypto_ahash_init_common(tfm, "sha512");
@@ -2762,53 +2706,6 @@ static struct ahash_alg hash_algos[] = {
 };
 
 static struct ahash_alg artpec7_hash_algos[] = {
-	/* SHA-384 */
-	{
-		.init = artpec6_crypto_sha384_init,
-		.update = artpec6_crypto_hash_update,
-		.final = artpec6_crypto_hash_final,
-		.digest = artpec6_crypto_sha384_digest,
-		.import = artpec6_crypto_hash_import,
-		.export = artpec6_crypto_hash_export,
-		.halg.digestsize = SHA384_DIGEST_SIZE,
-		.halg.statesize = sizeof(struct artpec6_hash_export_state),
-		.halg.base = {
-			.cra_name = "sha384",
-			.cra_driver_name = "artpec-sha384",
-			.cra_priority = 300,
-			.cra_flags = CRYPTO_ALG_ASYNC,
-			.cra_blocksize = SHA384_BLOCK_SIZE,
-			.cra_ctxsize = sizeof(struct artpec6_hashalg_context),
-			.cra_alignmask = 3,
-			.cra_module = THIS_MODULE,
-			.cra_init = artpec6_crypto_ahash_init,
-			.cra_exit = artpec6_crypto_ahash_exit,
-		}
-	},
-	/* HMAC SHA-384 */
-	{
-		.init = artpec6_crypto_hmac_sha384_init,
-		.update = artpec6_crypto_hash_update,
-		.final = artpec6_crypto_hash_final,
-		.digest = artpec6_crypto_hmac_sha384_digest,
-		.import = artpec6_crypto_hash_import,
-		.export = artpec6_crypto_hash_export,
-		.setkey = artpec6_crypto_hash_set_key,
-		.halg.digestsize = SHA384_DIGEST_SIZE,
-		.halg.statesize = sizeof(struct artpec6_hash_export_state),
-		.halg.base = {
-			.cra_name = "hmac(sha384)",
-			.cra_driver_name = "artpec-hmac-sha384",
-			.cra_priority = 300,
-			.cra_flags = CRYPTO_ALG_ASYNC,
-			.cra_blocksize = SHA384_BLOCK_SIZE,
-			.cra_ctxsize = sizeof(struct artpec6_hashalg_context),
-			.cra_alignmask = 3,
-			.cra_module = THIS_MODULE,
-			.cra_init = artpec6_crypto_ahash_init_hmac_sha384,
-			.cra_exit = artpec6_crypto_ahash_exit,
-		}
-	},
 	/* SHA-512 */
 	{
 		.init = artpec6_crypto_sha512_init,
-- 
2.11.0


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

* [PATCH 2/7] crypto: axis - remove sha512 support for artpec7
  2019-01-23 11:59 [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Lars Persson
  2019-01-23 11:59 ` [PATCH 1/7] crypto: axis - remove sha384 support for artpec7 Lars Persson
@ 2019-01-23 11:59 ` Lars Persson
  2019-01-23 11:59 ` [PATCH 3/7] crypto: axis - fix for recursive locking from bottom half Lars Persson
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Lars Persson @ 2019-01-23 11:59 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: Lars Persson

The hardware cannot restore the context correctly when it operates in
SHA512 mode. This is too restrictive when operating in a framework that
can interleave multiple hash sessions.

Signed-off-by: Lars Persson <larper@axis.com>
---
 drivers/crypto/axis/artpec6_crypto.c | 126 +++--------------------------------
 1 file changed, 9 insertions(+), 117 deletions(-)

diff --git a/drivers/crypto/axis/artpec6_crypto.c b/drivers/crypto/axis/artpec6_crypto.c
index e4cbb2d11514..43da19566a36 100644
--- a/drivers/crypto/axis/artpec6_crypto.c
+++ b/drivers/crypto/axis/artpec6_crypto.c
@@ -135,7 +135,6 @@
 #define regk_crypto_ext         0x00000001
 #define regk_crypto_hmac_sha1   0x00000007
 #define regk_crypto_hmac_sha256 0x00000009
-#define regk_crypto_hmac_sha512 0x0000000d
 #define regk_crypto_init        0x00000000
 #define regk_crypto_key_128     0x00000000
 #define regk_crypto_key_192     0x00000001
@@ -143,7 +142,6 @@
 #define regk_crypto_null        0x00000000
 #define regk_crypto_sha1        0x00000006
 #define regk_crypto_sha256      0x00000008
-#define regk_crypto_sha512      0x0000000c
 
 /* DMA descriptor structures */
 struct pdma_descr_ctrl  {
@@ -188,7 +186,6 @@ struct pdma_stat_descr {
 /* Hash modes (including HMAC variants) */
 #define ARTPEC6_CRYPTO_HASH_SHA1	1
 #define ARTPEC6_CRYPTO_HASH_SHA256	2
-#define ARTPEC6_CRYPTO_HASH_SHA512	4
 
 /* Crypto modes */
 #define ARTPEC6_CRYPTO_CIPHER_AES_ECB	1
@@ -288,11 +285,11 @@ struct artpec6_crypto_req_common {
 };
 
 struct artpec6_hash_request_context {
-	char partial_buffer[SHA512_BLOCK_SIZE];
-	char partial_buffer_out[SHA512_BLOCK_SIZE];
-	char key_buffer[SHA512_BLOCK_SIZE];
-	char pad_buffer[SHA512_BLOCK_SIZE + 32];
-	unsigned char digeststate[SHA512_DIGEST_SIZE];
+	char partial_buffer[SHA256_BLOCK_SIZE];
+	char partial_buffer_out[SHA256_BLOCK_SIZE];
+	char key_buffer[SHA256_BLOCK_SIZE];
+	char pad_buffer[SHA256_BLOCK_SIZE + 32];
+	unsigned char digeststate[SHA256_DIGEST_SIZE];
 	size_t partial_bytes;
 	u64 digcnt;
 	u32 key_md;
@@ -302,8 +299,8 @@ struct artpec6_hash_request_context {
 };
 
 struct artpec6_hash_export_state {
-	char partial_buffer[SHA512_BLOCK_SIZE];
-	unsigned char digeststate[SHA512_DIGEST_SIZE];
+	char partial_buffer[SHA256_BLOCK_SIZE];
+	unsigned char digeststate[SHA256_DIGEST_SIZE];
 	size_t partial_bytes;
 	u64 digcnt;
 	int oper;
@@ -311,7 +308,7 @@ struct artpec6_hash_export_state {
 };
 
 struct artpec6_hashalg_context {
-	char hmac_key[SHA512_BLOCK_SIZE];
+	char hmac_key[SHA256_BLOCK_SIZE];
 	size_t hmac_key_length;
 	struct crypto_shash *child_hash;
 };
@@ -2252,10 +2249,6 @@ artpec6_crypto_init_hash(struct ahash_request *req, u8 type, int hmac)
 	case ARTPEC6_CRYPTO_HASH_SHA256:
 		oper = hmac ? regk_crypto_hmac_sha256 : regk_crypto_sha256;
 		break;
-	case ARTPEC6_CRYPTO_HASH_SHA512:
-		oper = hmac ? regk_crypto_hmac_sha512 : regk_crypto_sha512;
-		break;
-
 	default:
 		pr_err("%s: Unsupported hash type 0x%x\n", MODULE_NAME, type);
 		return -EINVAL;
@@ -2351,31 +2344,11 @@ static int artpec6_crypto_sha256_digest(struct ahash_request *req)
 	return artpec6_crypto_prepare_submit_hash(req);
 }
 
-static int artpec6_crypto_sha512_init(struct ahash_request *req)
-{
-	return artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA512, 0);
-}
-
-static int artpec6_crypto_sha512_digest(struct ahash_request *req)
-{
-	struct artpec6_hash_request_context *req_ctx = ahash_request_ctx(req);
-
-	artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA512, 0);
-	req_ctx->hash_flags |= HASH_FLAG_UPDATE | HASH_FLAG_FINALIZE;
-
-	return artpec6_crypto_prepare_submit_hash(req);
-}
-
 static int artpec6_crypto_hmac_sha256_init(struct ahash_request *req)
 {
 	return artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA256, 1);
 }
 
-static int artpec6_crypto_hmac_sha512_init(struct ahash_request *req)
-{
-	return artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA512, 1);
-}
-
 static int artpec6_crypto_hmac_sha256_digest(struct ahash_request *req)
 {
 	struct artpec6_hash_request_context *req_ctx = ahash_request_ctx(req);
@@ -2386,16 +2359,6 @@ static int artpec6_crypto_hmac_sha256_digest(struct ahash_request *req)
 	return artpec6_crypto_prepare_submit_hash(req);
 }
 
-static int artpec6_crypto_hmac_sha512_digest(struct ahash_request *req)
-{
-	struct artpec6_hash_request_context *req_ctx = ahash_request_ctx(req);
-
-	artpec6_crypto_init_hash(req, ARTPEC6_CRYPTO_HASH_SHA512, 1);
-	req_ctx->hash_flags |= HASH_FLAG_UPDATE | HASH_FLAG_FINALIZE;
-
-	return artpec6_crypto_prepare_submit_hash(req);
-}
-
 static int artpec6_crypto_ahash_init_common(struct crypto_tfm *tfm,
 				    const char *base_hash_name)
 {
@@ -2430,11 +2393,6 @@ static int artpec6_crypto_ahash_init_hmac_sha256(struct crypto_tfm *tfm)
 	return artpec6_crypto_ahash_init_common(tfm, "sha256");
 }
 
-static int artpec6_crypto_ahash_init_hmac_sha512(struct crypto_tfm *tfm)
-{
-	return artpec6_crypto_ahash_init_common(tfm, "sha512");
-}
-
 static void artpec6_crypto_ahash_exit(struct crypto_tfm *tfm)
 {
 	struct artpec6_hashalg_context *tfm_ctx = crypto_tfm_ctx(tfm);
@@ -2705,56 +2663,6 @@ static struct ahash_alg hash_algos[] = {
 	},
 };
 
-static struct ahash_alg artpec7_hash_algos[] = {
-	/* SHA-512 */
-	{
-		.init = artpec6_crypto_sha512_init,
-		.update = artpec6_crypto_hash_update,
-		.final = artpec6_crypto_hash_final,
-		.digest = artpec6_crypto_sha512_digest,
-		.import = artpec6_crypto_hash_import,
-		.export = artpec6_crypto_hash_export,
-		.halg.digestsize = SHA512_DIGEST_SIZE,
-		.halg.statesize = sizeof(struct artpec6_hash_export_state),
-		.halg.base = {
-			.cra_name = "sha512",
-			.cra_driver_name = "artpec-sha512",
-			.cra_priority = 300,
-			.cra_flags = CRYPTO_ALG_ASYNC,
-			.cra_blocksize = SHA512_BLOCK_SIZE,
-			.cra_ctxsize = sizeof(struct artpec6_hashalg_context),
-			.cra_alignmask = 3,
-			.cra_module = THIS_MODULE,
-			.cra_init = artpec6_crypto_ahash_init,
-			.cra_exit = artpec6_crypto_ahash_exit,
-		}
-	},
-	/* HMAC SHA-512 */
-	{
-		.init = artpec6_crypto_hmac_sha512_init,
-		.update = artpec6_crypto_hash_update,
-		.final = artpec6_crypto_hash_final,
-		.digest = artpec6_crypto_hmac_sha512_digest,
-		.import = artpec6_crypto_hash_import,
-		.export = artpec6_crypto_hash_export,
-		.setkey = artpec6_crypto_hash_set_key,
-		.halg.digestsize = SHA512_DIGEST_SIZE,
-		.halg.statesize = sizeof(struct artpec6_hash_export_state),
-		.halg.base = {
-			.cra_name = "hmac(sha512)",
-			.cra_driver_name = "artpec-hmac-sha512",
-			.cra_priority = 300,
-			.cra_flags = CRYPTO_ALG_ASYNC,
-			.cra_blocksize = SHA512_BLOCK_SIZE,
-			.cra_ctxsize = sizeof(struct artpec6_hashalg_context),
-			.cra_alignmask = 3,
-			.cra_module = THIS_MODULE,
-			.cra_init = artpec6_crypto_ahash_init_hmac_sha512,
-			.cra_exit = artpec6_crypto_ahash_exit,
-		}
-	},
-};
-
 /* Crypto */
 static struct skcipher_alg crypto_algos[] = {
 	/* AES - ECB */
@@ -3001,19 +2909,10 @@ static int artpec6_crypto_probe(struct platform_device *pdev)
 		goto disable_hw;
 	}
 
-	if (variant != ARTPEC6_CRYPTO) {
-		err = crypto_register_ahashes(artpec7_hash_algos,
-					      ARRAY_SIZE(artpec7_hash_algos));
-		if (err) {
-			dev_err(dev, "Failed to register ahashes\n");
-			goto unregister_ahashes;
-		}
-	}
-
 	err = crypto_register_skciphers(crypto_algos, ARRAY_SIZE(crypto_algos));
 	if (err) {
 		dev_err(dev, "Failed to register ciphers\n");
-		goto unregister_a7_ahashes;
+		goto unregister_ahashes;
 	}
 
 	err = crypto_register_aeads(aead_algos, ARRAY_SIZE(aead_algos));
@@ -3026,10 +2925,6 @@ static int artpec6_crypto_probe(struct platform_device *pdev)
 
 unregister_algs:
 	crypto_unregister_skciphers(crypto_algos, ARRAY_SIZE(crypto_algos));
-unregister_a7_ahashes:
-	if (variant != ARTPEC6_CRYPTO)
-		crypto_unregister_ahashes(artpec7_hash_algos,
-					  ARRAY_SIZE(artpec7_hash_algos));
 unregister_ahashes:
 	crypto_unregister_ahashes(hash_algos, ARRAY_SIZE(hash_algos));
 disable_hw:
@@ -3045,9 +2940,6 @@ static int artpec6_crypto_remove(struct platform_device *pdev)
 	int irq = platform_get_irq(pdev, 0);
 
 	crypto_unregister_ahashes(hash_algos, ARRAY_SIZE(hash_algos));
-	if (ac->variant != ARTPEC6_CRYPTO)
-		crypto_unregister_ahashes(artpec7_hash_algos,
-					  ARRAY_SIZE(artpec7_hash_algos));
 	crypto_unregister_skciphers(crypto_algos, ARRAY_SIZE(crypto_algos));
 	crypto_unregister_aeads(aead_algos, ARRAY_SIZE(aead_algos));
 
-- 
2.11.0


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

* [PATCH 3/7] crypto: axis - fix for recursive locking from bottom half
  2019-01-23 11:59 [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Lars Persson
  2019-01-23 11:59 ` [PATCH 1/7] crypto: axis - remove sha384 support for artpec7 Lars Persson
  2019-01-23 11:59 ` [PATCH 2/7] crypto: axis - remove sha512 " Lars Persson
@ 2019-01-23 11:59 ` Lars Persson
  2019-01-23 11:59 ` [PATCH 4/7] crypto: axis - give DMA the start of the status buffer Lars Persson
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Lars Persson @ 2019-01-23 11:59 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: Lars Persson

Clients may submit a new requests from the completion callback
context. The driver was not prepared to receive a request in this
state because it already held the request queue lock and a recursive
lock error is triggered.

Now all completions are queued up until we are ready to drop the queue
lock and then delivered.

The fault was triggered by TCP over an IPsec connection in the LTP
test suite:
  LTP: starting tcp4_ipsec02 (tcp_ipsec.sh -p ah -m transport -s "100 1000 65535")
  BUG: spinlock recursion on CPU#1, genload/943
   lock: 0xbf3c3094, .magic: dead4ead, .owner: genload/943, .owner_cpu: 1
  CPU: 1 PID: 943 Comm: genload Tainted: G           O    4.9.62-axis5-devel #6
  Hardware name: Axis ARTPEC-6 Platform
   (unwind_backtrace) from [<8010d134>] (show_stack+0x18/0x1c)
   (show_stack) from [<803a289c>] (dump_stack+0x84/0x98)
   (dump_stack) from [<8016e164>] (do_raw_spin_lock+0x124/0x128)
   (do_raw_spin_lock) from [<804de1a4>] (artpec6_crypto_submit+0x2c/0xa0)
   (artpec6_crypto_submit) from [<804def38>] (artpec6_crypto_prepare_submit_hash+0xd0/0x54c)
   (artpec6_crypto_prepare_submit_hash) from [<7f3165f0>] (ah_output+0x2a4/0x3dc [ah4])
   (ah_output [ah4]) from [<805df9bc>] (xfrm_output_resume+0x178/0x4a4)
   (xfrm_output_resume) from [<805d283c>] (xfrm4_output+0xac/0xbc)
   (xfrm4_output) from [<80587928>] (ip_queue_xmit+0x140/0x3b4)
   (ip_queue_xmit) from [<805a13b4>] (tcp_transmit_skb+0x4c4/0x95c)
   (tcp_transmit_skb) from [<8059f218>] (tcp_rcv_state_process+0xdf4/0xdfc)
   (tcp_rcv_state_process) from [<805a7530>] (tcp_v4_do_rcv+0x64/0x1ac)
   (tcp_v4_do_rcv) from [<805a9724>] (tcp_v4_rcv+0xa34/0xb74)
   (tcp_v4_rcv) from [<80581d34>] (ip_local_deliver_finish+0x78/0x2b0)
   (ip_local_deliver_finish) from [<8058259c>] (ip_local_deliver+0xe4/0x104)
   (ip_local_deliver) from [<805d23ec>] (xfrm4_transport_finish+0xf4/0x144)
   (xfrm4_transport_finish) from [<805df564>] (xfrm_input+0x4f4/0x74c)
   (xfrm_input) from [<804de420>] (artpec6_crypto_task+0x208/0x38c)
   (artpec6_crypto_task) from [<801271b0>] (tasklet_action+0x60/0xec)
   (tasklet_action) from [<801266d4>] (__do_softirq+0xcc/0x3a4)
   (__do_softirq) from [<80126d20>] (irq_exit+0xf4/0x15c)
   (irq_exit) from [<801741e8>] (__handle_domain_irq+0x68/0xbc)
   (__handle_domain_irq) from [<801014f0>] (gic_handle_irq+0x50/0x94)
   (gic_handle_irq) from [<80657370>] (__irq_usr+0x50/0x80)

Signed-off-by: Lars Persson <larper@axis.com>
---
 drivers/crypto/axis/artpec6_crypto.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/axis/artpec6_crypto.c b/drivers/crypto/axis/artpec6_crypto.c
index 43da19566a36..675741067166 100644
--- a/drivers/crypto/axis/artpec6_crypto.c
+++ b/drivers/crypto/axis/artpec6_crypto.c
@@ -278,6 +278,7 @@ enum artpec6_crypto_hash_flags {
 
 struct artpec6_crypto_req_common {
 	struct list_head list;
+	struct list_head complete_in_progress;
 	struct artpec6_crypto_dma_descriptors *dma;
 	struct crypto_async_request *req;
 	void (*complete)(struct crypto_async_request *req);
@@ -2028,7 +2029,8 @@ static int artpec6_crypto_prepare_aead(struct aead_request *areq)
 	return artpec6_crypto_dma_map_descs(common);
 }
 
-static void artpec6_crypto_process_queue(struct artpec6_crypto *ac)
+static void artpec6_crypto_process_queue(struct artpec6_crypto *ac,
+	    struct list_head *completions)
 {
 	struct artpec6_crypto_req_common *req;
 
@@ -2039,7 +2041,7 @@ static void artpec6_crypto_process_queue(struct artpec6_crypto *ac)
 		list_move_tail(&req->list, &ac->pending);
 		artpec6_crypto_start_dma(req);
 
-		req->req->complete(req->req, -EINPROGRESS);
+		list_add_tail(&req->complete_in_progress, completions);
 	}
 
 	/*
@@ -2069,6 +2071,11 @@ static void artpec6_crypto_task(unsigned long data)
 	struct artpec6_crypto *ac = (struct artpec6_crypto *)data;
 	struct artpec6_crypto_req_common *req;
 	struct artpec6_crypto_req_common *n;
+	struct list_head complete_done;
+	struct list_head complete_in_progress;
+
+	INIT_LIST_HEAD(&complete_done);
+	INIT_LIST_HEAD(&complete_in_progress);
 
 	if (list_empty(&ac->pending)) {
 		pr_debug("Spurious IRQ\n");
@@ -2102,19 +2109,30 @@ static void artpec6_crypto_task(unsigned long data)
 
 		pr_debug("Completing request %p\n", req);
 
-		list_del(&req->list);
+		list_move_tail(&req->list, &complete_done);
 
 		artpec6_crypto_dma_unmap_all(req);
 		artpec6_crypto_copy_bounce_buffers(req);
 
 		ac->pending_count--;
 		artpec6_crypto_common_destroy(req);
-		req->complete(req->req);
 	}
 
-	artpec6_crypto_process_queue(ac);
+	artpec6_crypto_process_queue(ac, &complete_in_progress);
 
 	spin_unlock_bh(&ac->queue_lock);
+
+	/* Perform the completion callbacks without holding the queue lock
+	 * to allow new request submissions from the callbacks.
+	 */
+	list_for_each_entry_safe(req, n, &complete_done, list) {
+		req->complete(req->req);
+	}
+
+	list_for_each_entry_safe(req, n, &complete_in_progress,
+				 complete_in_progress) {
+		req->req->complete(req->req, -EINPROGRESS);
+	}
 }
 
 static void artpec6_crypto_complete_crypto(struct crypto_async_request *req)
-- 
2.11.0


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

* [PATCH 4/7] crypto: axis - give DMA the start of the status buffer
  2019-01-23 11:59 [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Lars Persson
                   ` (2 preceding siblings ...)
  2019-01-23 11:59 ` [PATCH 3/7] crypto: axis - fix for recursive locking from bottom half Lars Persson
@ 2019-01-23 11:59 ` Lars Persson
  2019-01-23 11:59 ` [PATCH 5/7] crypto: axis - support variable AEAD tag length Lars Persson
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Lars Persson @ 2019-01-23 11:59 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: Lars Persson

The driver was optimized to only do cache maintenance for the last
word of the dma descriptor status array. Unfortunately an omission
also passed the last word as the address of the array start to the DMA
engine. In most cases this goes unnoticed since the hardware aligns
the address to a 64 byte boundary.

Signed-off-by: Lars Persson <larper@axis.com>
---
 drivers/crypto/axis/artpec6_crypto.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/axis/artpec6_crypto.c b/drivers/crypto/axis/artpec6_crypto.c
index 675741067166..4936f8fb253a 100644
--- a/drivers/crypto/axis/artpec6_crypto.c
+++ b/drivers/crypto/axis/artpec6_crypto.c
@@ -665,8 +665,8 @@ artpec6_crypto_dma_map_descs(struct artpec6_crypto_req_common *common)
 	 * to be written.
 	 */
 	return artpec6_crypto_dma_map_single(common,
-				dma->stat + dma->in_cnt - 1,
-				sizeof(dma->stat[0]),
+				dma->stat,
+				sizeof(dma->stat[0]) * dma->in_cnt,
 				DMA_BIDIRECTIONAL,
 				&dma->stat_dma_addr);
 }
@@ -2087,9 +2087,12 @@ static void artpec6_crypto_task(unsigned long data)
 	list_for_each_entry_safe(req, n, &ac->pending, list) {
 		struct artpec6_crypto_dma_descriptors *dma = req->dma;
 		u32 stat;
+		dma_addr_t stataddr;
 
-		dma_sync_single_for_cpu(artpec6_crypto_dev, dma->stat_dma_addr,
-					sizeof(dma->stat[0]),
+		stataddr = dma->stat_dma_addr + 4 * (req->dma->in_cnt - 1);
+		dma_sync_single_for_cpu(artpec6_crypto_dev,
+					stataddr,
+					4,
 					DMA_BIDIRECTIONAL);
 
 		stat = req->dma->stat[req->dma->in_cnt-1];
-- 
2.11.0


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

* [PATCH 5/7] crypto: axis - support variable AEAD tag length
  2019-01-23 11:59 [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Lars Persson
                   ` (3 preceding siblings ...)
  2019-01-23 11:59 ` [PATCH 4/7] crypto: axis - give DMA the start of the status buffer Lars Persson
@ 2019-01-23 11:59 ` Lars Persson
  2019-01-23 11:59 ` [PATCH 6/7] crypto: axis - use a constant time tag compare Lars Persson
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Lars Persson @ 2019-01-23 11:59 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: Lars Persson

The implementation assumed that the client always wants the whole 16
byte AES-GCM tag. Now we respect the requested authentication tag size
fetched using crypto_aead_authsize().

Signed-off-by: Lars Persson <larper@axis.com>
---
 drivers/crypto/axis/artpec6_crypto.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/axis/artpec6_crypto.c b/drivers/crypto/axis/artpec6_crypto.c
index 4936f8fb253a..1be5bdd658a4 100644
--- a/drivers/crypto/axis/artpec6_crypto.c
+++ b/drivers/crypto/axis/artpec6_crypto.c
@@ -1907,7 +1907,7 @@ static int artpec6_crypto_prepare_aead(struct aead_request *areq)
 	/* For the decryption, cryptlen includes the tag. */
 	input_length = areq->cryptlen;
 	if (req_ctx->decrypt)
-		input_length -= AES_BLOCK_SIZE;
+		input_length -= crypto_aead_authsize(cipher);
 
 	/* Prepare the context buffer */
 	req_ctx->hw_ctx.aad_length_bits =
@@ -1972,7 +1972,7 @@ static int artpec6_crypto_prepare_aead(struct aead_request *areq)
 		size_t output_len = areq->cryptlen;
 
 		if (req_ctx->decrypt)
-			output_len -= AES_BLOCK_SIZE;
+			output_len -= crypto_aead_authsize(cipher);
 
 		artpec6_crypto_walk_init(&walk, areq->dst);
 
@@ -2001,19 +2001,32 @@ static int artpec6_crypto_prepare_aead(struct aead_request *areq)
 		 * the output ciphertext. For decryption it is put in a context
 		 * buffer for later compare against the input tag.
 		 */
-		count = AES_BLOCK_SIZE;
 
 		if (req_ctx->decrypt) {
 			ret = artpec6_crypto_setup_in_descr(common,
-				req_ctx->decryption_tag, count, false);
+				req_ctx->decryption_tag, AES_BLOCK_SIZE, false);
 			if (ret)
 				return ret;
 
 		} else {
+			/* For encryption the requested tag size may be smaller
+			 * than the hardware's generated tag.
+			 */
+			size_t authsize = crypto_aead_authsize(cipher);
+
 			ret = artpec6_crypto_setup_sg_descrs_in(common, &walk,
-								count);
+								authsize);
 			if (ret)
 				return ret;
+
+			if (authsize < AES_BLOCK_SIZE) {
+				count = AES_BLOCK_SIZE - authsize;
+				ret = artpec6_crypto_setup_in_descr(common,
+					ac->pad_buffer,
+					count, false);
+				if (ret)
+					return ret;
+			}
 		}
 
 	}
@@ -2174,27 +2187,29 @@ static void artpec6_crypto_complete_aead(struct crypto_async_request *req)
 	/* Verify GCM hashtag. */
 	struct aead_request *areq = container_of(req,
 		struct aead_request, base);
+	struct crypto_aead *aead = crypto_aead_reqtfm(areq);
 	struct artpec6_crypto_aead_req_ctx *req_ctx = aead_request_ctx(areq);
 
 	if (req_ctx->decrypt) {
 		u8 input_tag[AES_BLOCK_SIZE];
+		unsigned int authsize = crypto_aead_authsize(aead);
 
 		sg_pcopy_to_buffer(areq->src,
 				   sg_nents(areq->src),
 				   input_tag,
-				   AES_BLOCK_SIZE,
+				   authsize,
 				   areq->assoclen + areq->cryptlen -
-				   AES_BLOCK_SIZE);
+				   authsize);
 
 		if (memcmp(req_ctx->decryption_tag,
 			   input_tag,
-			   AES_BLOCK_SIZE)) {
+			   authsize)) {
 			pr_debug("***EBADMSG:\n");
 			print_hex_dump_debug("ref:", DUMP_PREFIX_ADDRESS, 32, 1,
-					     input_tag, AES_BLOCK_SIZE, true);
+					     input_tag, authsize, true);
 			print_hex_dump_debug("out:", DUMP_PREFIX_ADDRESS, 32, 1,
 					     req_ctx->decryption_tag,
-					     AES_BLOCK_SIZE, true);
+					     authsize, true);
 
 			result = -EBADMSG;
 		}
-- 
2.11.0


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

* [PATCH 6/7] crypto: axis - use a constant time tag compare
  2019-01-23 11:59 [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Lars Persson
                   ` (4 preceding siblings ...)
  2019-01-23 11:59 ` [PATCH 5/7] crypto: axis - support variable AEAD tag length Lars Persson
@ 2019-01-23 11:59 ` Lars Persson
  2019-01-23 11:59 ` [PATCH 7/7] crypto: axis - move request unmap outside of the queue lock Lars Persson
  2019-02-01  6:49 ` [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Herbert Xu
  7 siblings, 0 replies; 9+ messages in thread
From: Lars Persson @ 2019-01-23 11:59 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: Lars Persson

Avoid plain memcmp() on the AEAD tag value as this could leak
information through a timing side channel.

Signed-off-by: Lars Persson <larper@axis.com>
---
 drivers/crypto/axis/artpec6_crypto.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/axis/artpec6_crypto.c b/drivers/crypto/axis/artpec6_crypto.c
index 1be5bdd658a4..71ef9ce68fd8 100644
--- a/drivers/crypto/axis/artpec6_crypto.c
+++ b/drivers/crypto/axis/artpec6_crypto.c
@@ -2201,9 +2201,9 @@ static void artpec6_crypto_complete_aead(struct crypto_async_request *req)
 				   areq->assoclen + areq->cryptlen -
 				   authsize);
 
-		if (memcmp(req_ctx->decryption_tag,
-			   input_tag,
-			   authsize)) {
+		if (crypto_memneq(req_ctx->decryption_tag,
+				  input_tag,
+				  authsize)) {
 			pr_debug("***EBADMSG:\n");
 			print_hex_dump_debug("ref:", DUMP_PREFIX_ADDRESS, 32, 1,
 					     input_tag, authsize, true);
-- 
2.11.0


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

* [PATCH 7/7] crypto: axis - move request unmap outside of the queue lock
  2019-01-23 11:59 [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Lars Persson
                   ` (5 preceding siblings ...)
  2019-01-23 11:59 ` [PATCH 6/7] crypto: axis - use a constant time tag compare Lars Persson
@ 2019-01-23 11:59 ` Lars Persson
  2019-02-01  6:49 ` [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Herbert Xu
  7 siblings, 0 replies; 9+ messages in thread
From: Lars Persson @ 2019-01-23 11:59 UTC (permalink / raw)
  To: linux-crypto, herbert; +Cc: Vincent Whitchurch, Lars Persson

From: Vincent Whitchurch <rabinv@axis.com>

The request unmap and bounce buffer copying is currently unnecessarily
done while holding the queue spin lock.

Signed-off-by: Lars Persson <larper@axis.com>
Signed-off-by: Vincent Whitchurch <rabinv@axis.com>
---
 drivers/crypto/axis/artpec6_crypto.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/axis/artpec6_crypto.c b/drivers/crypto/axis/artpec6_crypto.c
index 71ef9ce68fd8..47b76cf2c764 100644
--- a/drivers/crypto/axis/artpec6_crypto.c
+++ b/drivers/crypto/axis/artpec6_crypto.c
@@ -2127,11 +2127,7 @@ static void artpec6_crypto_task(unsigned long data)
 
 		list_move_tail(&req->list, &complete_done);
 
-		artpec6_crypto_dma_unmap_all(req);
-		artpec6_crypto_copy_bounce_buffers(req);
-
 		ac->pending_count--;
-		artpec6_crypto_common_destroy(req);
 	}
 
 	artpec6_crypto_process_queue(ac, &complete_in_progress);
@@ -2142,6 +2138,10 @@ static void artpec6_crypto_task(unsigned long data)
 	 * to allow new request submissions from the callbacks.
 	 */
 	list_for_each_entry_safe(req, n, &complete_done, list) {
+		artpec6_crypto_dma_unmap_all(req);
+		artpec6_crypto_copy_bounce_buffers(req);
+		artpec6_crypto_common_destroy(req);
+
 		req->complete(req->req);
 	}
 
-- 
2.11.0


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

* Re: [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs
  2019-01-23 11:59 [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Lars Persson
                   ` (6 preceding siblings ...)
  2019-01-23 11:59 ` [PATCH 7/7] crypto: axis - move request unmap outside of the queue lock Lars Persson
@ 2019-02-01  6:49 ` Herbert Xu
  7 siblings, 0 replies; 9+ messages in thread
From: Herbert Xu @ 2019-02-01  6:49 UTC (permalink / raw)
  To: Lars Persson; +Cc: linux-crypto, Lars Persson

On Wed, Jan 23, 2019 at 12:59:39PM +0100, Lars Persson wrote:
> Hi
> 
> This series brings to mainline fixes done during our product development and
> fixes for errors detected by the IPsec testsuite in LTP.
> 
> Lars Persson (6):
>   crypto: axis - remove sha384 support for artpec7
>   crypto: axis - remove sha512 support for artpec7
>   crypto: axis - fix for recursive locking from bottom half
>   crypto: axis - give DMA the start of the status buffer
>   crypto: axis - support variable AEAD tag length
>   crypto: axis - use a constant time tag compare
> 
> Vincent Whitchurch (1):
>   crypto: axis - move request unmap outside of the queue lock
> 
>  drivers/crypto/axis/artpec6_crypto.c | 317 ++++++++---------------------------
>  1 file changed, 71 insertions(+), 246 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:[~2019-02-01  6:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-23 11:59 [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Lars Persson
2019-01-23 11:59 ` [PATCH 1/7] crypto: axis - remove sha384 support for artpec7 Lars Persson
2019-01-23 11:59 ` [PATCH 2/7] crypto: axis - remove sha512 " Lars Persson
2019-01-23 11:59 ` [PATCH 3/7] crypto: axis - fix for recursive locking from bottom half Lars Persson
2019-01-23 11:59 ` [PATCH 4/7] crypto: axis - give DMA the start of the status buffer Lars Persson
2019-01-23 11:59 ` [PATCH 5/7] crypto: axis - support variable AEAD tag length Lars Persson
2019-01-23 11:59 ` [PATCH 6/7] crypto: axis - use a constant time tag compare Lars Persson
2019-01-23 11:59 ` [PATCH 7/7] crypto: axis - move request unmap outside of the queue lock Lars Persson
2019-02-01  6:49 ` [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs Herbert Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.