2019-01-23 11:59:53

by Lars Persson

[permalink] [raw]
Subject: [PATCH 0/7] 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(-)

--
2.11.0



2019-01-23 11:59:56

by Lars Persson

[permalink] [raw]
Subject: [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(-)

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


2019-01-23 11:59:58

by Lars Persson

[permalink] [raw]
Subject: [PATCH 5/7] crypto: axis - support variable AEAD tag length

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

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


2019-01-23 11:59:58

by Lars Persson

[permalink] [raw]
Subject: [PATCH 3/7] crypto: axis - fix for recursive locking from bottom half

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

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


2019-01-23 12:00:00

by Lars Persson

[permalink] [raw]
Subject: [PATCH 7/7] crypto: axis - move request unmap outside of the queue lock

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

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


2019-01-23 12:00:00

by Lars Persson

[permalink] [raw]
Subject: [PATCH 1/7] crypto: axis - remove sha384 support for artpec7

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

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


2019-01-23 12:00:01

by Lars Persson

[permalink] [raw]
Subject: [PATCH 4/7] crypto: axis - give DMA the start of the status buffer

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

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


2019-01-23 12:00:02

by Lars Persson

[permalink] [raw]
Subject: [PATCH 6/7] crypto: axis - use a constant time tag compare

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

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


2019-02-01 06:50:04

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/7] crypto: axis - fixes for the Artpec SoCs

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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt