Skip to contentSkip to conversation

crypto: axis - fixes for the Artpec SoCs

PATCH In review Lars Persson 2 participants 9 messages Jan 23, 2019 – Feb 1, 2019

Subsystems (2)
Crypto API Arm/artpec machine support
lore
Share & tools
lore

Lars Persson

author maintainer
#0 lore raw

cover crypto: axis - fixes for the Artpec SoCs

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(-)
Signature · 1 line
2.11.0
author maintainer
#2 lore raw

PATCH 2/7 crypto: axis - remove sha512 support for artpec7

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 <[email protected]>
---
 drivers/crypto/axis/artpec6_crypto.c | 126 +++--------------------------------
 1 file changed, 9 insertions(+), 117 deletions(-)
drivers/crypto/axis/artpec6_crypto.c +9 -117
index e4cbb2d11514..43da19566a36 100644
@@ -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));
Signature · 1 line
2.11.0
author maintainer
#3 lore raw

PATCH 3/7 crypto: axis - fix for recursive locking from bottom half

In reply to 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 <[email protected]>
---
 drivers/crypto/axis/artpec6_crypto.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)
drivers/crypto/axis/artpec6_crypto.c +23 -5
index 43da19566a36..675741067166 100644
@@ -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)
Signature · 1 line
2.11.0
author maintainer
#5 lore raw

PATCH 5/7 crypto: axis - support variable AEAD tag length

In reply to 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 <[email protected]>
---
 drivers/crypto/axis/artpec6_crypto.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)
drivers/crypto/axis/artpec6_crypto.c +25 -10
index 4936f8fb253a..1be5bdd658a4 100644
@@ -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;
 		}
Signature · 1 line
2.11.0
author maintainer
#1 lore raw

PATCH 1/7 crypto: axis - remove sha384 support for artpec7

In reply to Lars Persson

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

Signed-off-by Lars Persson <[email protected]>
---
 drivers/crypto/axis/artpec6_crypto.c | 107 +----------------------------------
 1 file changed, 2 insertions(+), 105 deletions(-)
drivers/crypto/axis/artpec6_crypto.c +2 -105
index f3442c2bdbdc..e4cbb2d11514 100644
@@ -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,
Signature · 1 line
2.11.0
author maintainer
#7 lore raw

PATCH 7/7 crypto: axis - move request unmap outside of the queue lock

In reply to Lars Persson

From: Vincent Whitchurch <[email protected]>

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

Signed-off-by Lars Persson <[email protected]>
Signed-off-by Vincent Whitchurch <[email protected]>
---
 drivers/crypto/axis/artpec6_crypto.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/crypto/axis/artpec6_crypto.c +4 -4
index 71ef9ce68fd8..47b76cf2c764 100644
@@ -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);
 	}
Signature · 1 line
2.11.0
author maintainer
#4 lore raw

PATCH 4/7 crypto: axis - give DMA the start of the status buffer

In reply to 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 <[email protected]>
---
 drivers/crypto/axis/artpec6_crypto.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/crypto/axis/artpec6_crypto.c +7 -4
index 675741067166..4936f8fb253a 100644
@@ -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];
Signature · 1 line
2.11.0
author maintainer
#6 lore raw

PATCH 6/7 crypto: axis - use a constant time tag compare

In reply to 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 <[email protected]>
---
 drivers/crypto/axis/artpec6_crypto.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/crypto/axis/artpec6_crypto.c +3 -3
index 1be5bdd658a4..71ef9ce68fd8 100644
@@ -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);
Signature · 1 line
2.11.0
maintainer
#8 lore raw
In reply to Lars Persson
Quoted 13 lines · 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

Quoted 5 lines

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.

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt