[PATCH 0/6] crypto: talitos - various fixes

[PATCH 0/6] crypto: talitos - various fixes

[Christophe Leroy]

This serie provide various fixes on the talitos driver.

Christophe Leroy (6):
  crypto: talitos - Don't provide setkey for non hmac hashing algs.
  crypto: talitos - fix hashing
  crypto: talitos - fix sha224
  crypto: talitos - fix AEAD test failures
  crypto: talitos - use kzalloc instead of kmalloc
  crypto: talitos - fix memory corruption on SEC2

 drivers/crypto/talitos.c | 81 +++++++++++++++++++++---------------------------
 drivers/crypto/talitos.h |  2 --
 2 files changed, 36 insertions(+), 47 deletions(-)

-- 
2.13.3

[PATCH 1/6] crypto: talitos - Don't provide setkey for non hmac hashing algs.

[Christophe Leroy]

Today, md5sum fails with error -ENOKEY because a setkey
function is set for non hmac hashing algs, see strace output below:

mmap(NULL, 378880, PROT_READ, MAP_SHARED, 6, 0) = 0x77f50000
accept(3, 0, NULL)                      = 7
vmsplice(5, [{"bin/\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 378880}], 1, SPLICE_F_MORE|SPLICE_F_GIFT) = 262144
splice(4, NULL, 7, NULL, 262144, SPLICE_F_MORE) = -1 ENOKEY (Required key not available)
write(2, "Generation of hash for file kcap"..., 50) = 50
munmap(0x77f50000, 378880)              = 0

This patch ensures that setkey() function is set only
for hmac hashing.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/crypto/talitos.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 79791c690858..5cc160078286 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -3057,7 +3057,8 @@ static struct talitos_crypto_alg *talitos_alg_alloc(struct device *dev,
 		t_alg->algt.alg.hash.final = ahash_final;
 		t_alg->algt.alg.hash.finup = ahash_finup;
 		t_alg->algt.alg.hash.digest = ahash_digest;
-		t_alg->algt.alg.hash.setkey = ahash_setkey;
+		if (!strncmp(alg->cra_name, "hmac", 4))
+			t_alg->algt.alg.hash.setkey = ahash_setkey;
 		t_alg->algt.alg.hash.import = ahash_import;
 		t_alg->algt.alg.hash.export = ahash_export;
 
-- 
2.13.3

[PATCH 2/6] crypto: talitos - fix hashing

[Christophe Leroy]

md5sum on some files gives wrong result

Exemple:

With the md5sum from libkcapi:
c15115c05bad51113f81bdaee735dd09  test

With the original md5sum:
bbdf41d80ba7e8b2b7be3a0772be76cb  test

This patch fixes this issue

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/crypto/talitos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 5cc160078286..49f1561fa694 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1769,7 +1769,7 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 
 	sg_count = edesc->src_nents ?: 1;
 	if (is_sec1 && sg_count > 1)
-		sg_copy_to_buffer(areq->src, sg_count, edesc->buf, length);
+		sg_copy_to_buffer(req_ctx->psrc, sg_count, edesc->buf, length);
 	else
 		sg_count = dma_map_sg(dev, req_ctx->psrc, sg_count,
 				      DMA_TO_DEVICE);
-- 
2.13.3

[PATCH 3/6] crypto: talitos - fix sha224

[Christophe Leroy]

Kernel crypto tests report the following error at startup

[    2.752626] alg: hash: Test 4 failed for sha224-talitos
[    2.757907] 00000000: 30 e2 86 e2 e7 8a dd 0d d7 eb 9f d5 83 fe f1 b0
00000010: 2d 5a 6c a5 f9 55 ea fd 0e 72 05 22

This patch fixes it

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/crypto/talitos.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 49f1561fa694..dff88838dce7 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1756,9 +1756,9 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 		req_ctx->swinit = 0;
 	} else {
 		desc->ptr[1] = zero_entry;
-		/* Indicate next op is not the first. */
-		req_ctx->first = 0;
 	}
+	/* Indicate next op is not the first. */
+	req_ctx->first = 0;
 
 	/* HMAC key */
 	if (ctx->keylen)
-- 
2.13.3

[PATCH 4/6] crypto: talitos - fix AEAD test failures

[Christophe Leroy]

AEAD tests fail when destination SG list has more than 1 element.

[    2.058752] alg: aead: Test 1 failed on encryption for authenc-hmac-sha1-cbc-aes-talitos
[    2.066965] 00000000: 53 69 6e 67 6c 65 20 62 6c 6f 63 6b 20 6d 73 67
00000010: c0 43 ff 74 c0 43 ff e0 de 83 d1 20 de 84 8e 54
00000020: de 83 d7 c4
[    2.082138] alg: aead: Test 1 failed on encryption for authenc-hmac-sha1-cbc-aes-talitos
[    2.090435] 00000000: 53 69 6e 67 6c 65 20 62 6c 6f 63 6b 20 6d 73 67
00000010: de 84 ea 58 c0 93 1a 24 de 84 e8 59 de 84 f1 20
00000020: 00 00 00 00
[    2.105721] alg: aead: Test 1 failed on encryption for authenc-hmac-sha1-cbc-3des-talitos
[    2.114259] 00000000: 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72 73 74
00000010: 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63 74 65
00000020: 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72
00000030: 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63
00000040: 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65
00000050: 65 72 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53
00000060: 72 63 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20
00000070: 63 65 65 72 73 74 54 20 6f 6f 4d 20 6e 61 0a 79
00000080: c0 50 f1 ac c0 50 f3 38 c0 50 f3 94 c0 50 f5 30
00000090: c0 99 74 3c
[    2.166410] alg: aead: Test 1 failed on encryption for authenc-hmac-sha1-cbc-3des-talitos
[    2.174794] 00000000: 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72 73 74
00000010: 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63 74 65
00000020: 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72
00000030: 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63
00000040: 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65
00000050: 65 72 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53
00000060: 72 63 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20
00000070: 63 65 65 72 73 74 54 20 6f 6f 4d 20 6e 61 0a 79
00000080: c0 50 f1 ac c0 50 f3 38 c0 50 f3 94 c0 50 f5 30
00000090: c0 99 74 3c
[    2.226486] alg: No test for authenc(hmac(sha224),cbc(aes)) (authenc-hmac-sha224-cbc-aes-talitos)
[    2.236459] alg: No test for authenc(hmac(sha224),cbc(aes)) (authenc-hmac-sha224-cbc-aes-talitos)
[    2.247196] alg: aead: Test 1 failed on encryption for authenc-hmac-sha224-cbc-3des-talitos
[    2.255555] 00000000: 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72 73 74
00000010: 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63 74 65
00000020: 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72
00000030: 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63
00000040: 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65
00000050: 65 72 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53
00000060: 72 63 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20
00000070: 63 65 65 72 73 74 54 20 6f 6f 4d 20 6e 61 0a 79
00000080: c0 50 f1 ac c0 50 f3 38 c0 50 f3 94 c0 50 f5 30
00000090: c0 99 74 3c c0 96 e5 b8
[    2.309004] alg: aead: Test 1 failed on encryption for authenc-hmac-sha224-cbc-3des-talitos
[    2.317562] 00000000: 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72 73 74
00000010: 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63 74 65
00000020: 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72
00000030: 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63
00000040: 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65
00000050: 65 72 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53
00000060: 72 63 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20
00000070: 63 65 65 72 73 74 54 20 6f 6f 4d 20 6e 61 0a 79
00000080: c0 50 f1 ac c0 50 f3 38 c0 50 f3 94 c0 50 f5 30
00000090: c0 99 74 3c c0 96 e5 b8
[    2.370710] alg: aead: Test 1 failed on encryption for authenc-hmac-sha256-cbc-aes-talitos
[    2.379177] 00000000: 53 69 6e 67 6c 65 20 62 6c 6f 63 6b 20 6d 73 67
00000010: 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63 74 65
00000020: 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72
[    2.397863] alg: aead: Test 1 failed on encryption for authenc-hmac-sha256-cbc-aes-talitos
[    2.406134] 00000000: 53 69 6e 67 6c 65 20 62 6c 6f 63 6b 20 6d 73 67
00000010: 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63 74 65
00000020: 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72
[    2.424789] alg: aead: Test 1 failed on encryption for authenc-hmac-sha256-cbc-3des-talitos
[    2.433491] 00000000: 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72 73 74
00000010: 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63 74 65
00000020: 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72
00000030: 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63
00000040: 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65
00000050: 65 72 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53
00000060: 72 63 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20
00000070: 63 65 65 72 73 74 54 20 6f 6f 4d 20 6e 61 0a 79
00000080: c0 50 f1 ac c0 50 f3 38 c0 50 f3 94 c0 50 f5 30
00000090: c0 99 74 3c c0 96 e5 b8 c0 96 e9 20 c0 00 3d dc
[    2.488832] alg: aead: Test 1 failed on encryption for authenc-hmac-sha256-cbc-3des-talitos
[    2.497387] 00000000: 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72 73 74
00000010: 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63 74 65
00000020: 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65 65 72
00000030: 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53 72 63
00000040: 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20 63 65
00000050: 65 72 73 74 54 20 6f 6f 4d 20 6e 61 20 79 65 53
00000060: 72 63 74 65 20 73 6f 54 20 6f 61 4d 79 6e 53 20
00000070: 63 65 65 72 73 74 54 20 6f 6f 4d 20 6e 61 0a 79
00000080: c0 50 f1 ac c0 50 f3 38 c0 50 f3 94 c0 50 f5 30
00000090: c0 99 74 3c c0 96 e5 b8 c0 96 e9 20 c0 00 3d dc

This patch fixes that.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/crypto/talitos.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index dff88838dce7..cd8a37e60259 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1232,12 +1232,11 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
 			sg_link_tbl_len += authsize;
 	}
 
-	sg_count = talitos_sg_map(dev, areq->src, cryptlen, edesc,
-				  &desc->ptr[4], sg_count, areq->assoclen,
-				  tbl_off);
+	ret = talitos_sg_map(dev, areq->src, cryptlen, edesc, &desc->ptr[4],
+			     sg_count, areq->assoclen, tbl_off);
 
-	if (sg_count > 1) {
-		tbl_off += sg_count;
+	if (ret > 1) {
+		tbl_off += ret;
 		sync_needed = true;
 	}
 
-- 
2.13.3

[PATCH 5/6] crypto: talitos - use kzalloc instead of kmalloc

[Christophe Leroy]

Use kzalloc() to zeroize the extended descriptor at allocation and
further zeorising

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/crypto/talitos.c | 23 +----------------------
 drivers/crypto/talitos.h |  2 --
 2 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index cd8a37e60259..a5b608b54c74 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -75,7 +75,6 @@ static void to_talitos_ptr_len(struct talitos_ptr *ptr, unsigned int len,
 			       bool is_sec1)
 {
 	if (is_sec1) {
-		ptr->res = 0;
 		ptr->len1 = cpu_to_be16(len);
 	} else {
 		ptr->len = cpu_to_be16(len);
@@ -118,7 +117,6 @@ static void map_single_talitos_ptr(struct device *dev,
 
 	to_talitos_ptr_len(ptr, len, is_sec1);
 	to_talitos_ptr(ptr, dma_addr, is_sec1);
-	to_talitos_ptr_ext_set(ptr, 0, is_sec1);
 }
 
 /*
@@ -287,7 +285,6 @@ int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,
 	/* map descriptor and save caller data */
 	if (is_sec1) {
 		desc->hdr1 = desc->hdr;
-		desc->next_desc = 0;
 		request->dma_desc = dma_map_single(dev, &desc->hdr1,
 						   TALITOS_DESC_SIZE,
 						   DMA_BIDIRECTIONAL);
@@ -1099,7 +1096,6 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
 		to_talitos_ptr(link_tbl_ptr + count,
 			       sg_dma_address(sg) + offset, 0);
 		to_talitos_ptr_len(link_tbl_ptr + count, len, 0);
-		to_talitos_ptr_ext_set(link_tbl_ptr + count, 0, 0);
 		count++;
 		cryptlen -= len;
 		offset = 0;
@@ -1125,7 +1121,6 @@ int talitos_sg_map(struct device *dev, struct scatterlist *src,
 	bool is_sec1 = has_ftr_sec1(priv);
 
 	to_talitos_ptr_len(ptr, len, is_sec1);
-	to_talitos_ptr_ext_set(ptr, 0, is_sec1);
 
 	if (sg_count == 1) {
 		to_talitos_ptr(ptr, sg_dma_address(src) + offset, is_sec1);
@@ -1197,11 +1192,9 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
 	if (desc->hdr & DESC_HDR_TYPE_IPSEC_ESP) {
 		to_talitos_ptr(&desc->ptr[2], edesc->iv_dma, is_sec1);
 		to_talitos_ptr_len(&desc->ptr[2], ivsize, is_sec1);
-		to_talitos_ptr_ext_set(&desc->ptr[2], 0, is_sec1);
 	} else {
 		to_talitos_ptr(&desc->ptr[3], edesc->iv_dma, is_sec1);
 		to_talitos_ptr_len(&desc->ptr[3], ivsize, is_sec1);
-		to_talitos_ptr_ext_set(&desc->ptr[3], 0, is_sec1);
 	}
 
 	/* cipher key */
@@ -1221,7 +1214,6 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
 	 * typically 12 for ipsec
 	 */
 	to_talitos_ptr_len(&desc->ptr[4], cryptlen, is_sec1);
-	to_talitos_ptr_ext_set(&desc->ptr[4], 0, is_sec1);
 
 	sg_link_tbl_len = cryptlen;
 
@@ -1275,8 +1267,6 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
 			to_talitos_ptr(tbl_ptr, edesc->dma_link_tbl + offset,
 				       is_sec1);
 		}
-	} else {
-		edesc->icv_ool = false;
 	}
 
 	/* ICV data */
@@ -1386,7 +1376,7 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
 		alloc_len += icv_stashing ? authsize : 0;
 	}
 
-	edesc = kmalloc(alloc_len, GFP_DMA | flags);
+	edesc = kzalloc(alloc_len, GFP_DMA | flags);
 	if (!edesc) {
 		dev_err(dev, "could not allocate edescriptor\n");
 		err = ERR_PTR(-ENOMEM);
@@ -1467,7 +1457,6 @@ static int aead_decrypt(struct aead_request *req)
 				  DESC_HDR_MODE1_MDEU_CICV;
 
 		/* reset integrity check result bits */
-		edesc->desc.hdr_lo = 0;
 
 		return ipsec_esp(edesc, req, ipsec_esp_decrypt_hwauth_done);
 	}
@@ -1554,12 +1543,10 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
 	bool is_sec1 = has_ftr_sec1(priv);
 
 	/* first DWORD empty */
-	desc->ptr[0] = zero_entry;
 
 	/* cipher iv */
 	to_talitos_ptr(&desc->ptr[1], edesc->iv_dma, is_sec1);
 	to_talitos_ptr_len(&desc->ptr[1], ivsize, is_sec1);
-	to_talitos_ptr_ext_set(&desc->ptr[1], 0, is_sec1);
 
 	/* cipher key */
 	map_single_talitos_ptr(dev, &desc->ptr[2], ctx->keylen,
@@ -1598,7 +1585,6 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
 			       DMA_FROM_DEVICE);
 
 	/* last DWORD empty */
-	desc->ptr[6] = zero_entry;
 
 	if (sync_needed)
 		dma_sync_single_for_device(dev, edesc->dma_link_tbl,
@@ -1744,7 +1730,6 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 	int sg_count;
 
 	/* first DWORD empty */
-	desc->ptr[0] = zero_entry;
 
 	/* hash context in */
 	if (!req_ctx->first || req_ctx->swinit) {
@@ -1753,8 +1738,6 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 				       (char *)req_ctx->hw_context,
 				       DMA_TO_DEVICE);
 		req_ctx->swinit = 0;
-	} else {
-		desc->ptr[1] = zero_entry;
 	}
 	/* Indicate next op is not the first. */
 	req_ctx->first = 0;
@@ -1763,8 +1746,6 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 	if (ctx->keylen)
 		map_single_talitos_ptr(dev, &desc->ptr[2], ctx->keylen,
 				       (char *)&ctx->key, DMA_TO_DEVICE);
-	else
-		desc->ptr[2] = zero_entry;
 
 	sg_count = edesc->src_nents ?: 1;
 	if (is_sec1 && sg_count > 1)
@@ -1781,7 +1762,6 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 		sync_needed = true;
 
 	/* fifth DWORD empty */
-	desc->ptr[4] = zero_entry;
 
 	/* hash/HMAC out -or- hash context out */
 	if (req_ctx->last)
@@ -1794,7 +1774,6 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 				       req_ctx->hw_context, DMA_FROM_DEVICE);
 
 	/* last DWORD empty */
-	desc->ptr[6] = zero_entry;
 
 	if (is_sec1 && from_talitos_ptr_len(&desc->ptr[3], true) == 0)
 		talitos_handle_buggy_hash(ctx, edesc, &desc->ptr[3]);
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index 8dd8f40e2771..6112ff1fc334 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -52,8 +52,6 @@ struct talitos_ptr {
 	__be32 ptr;     /* address */
 };
 
-static const struct talitos_ptr zero_entry;
-
 /* descriptor */
 struct talitos_desc {
 	__be32 hdr;                     /* header high bits */
-- 
2.13.3

[PATCH 6/6] crypto: talitos - fix memory corruption on SEC2

[Christophe Leroy]

On SEC2, when using the old descriptors type (hmac snoop no afeu)
for doing IPsec, the CICV out pointeur points out of the allocated
memory.

[    2.502554] =============================================================================
[    2.510740] BUG dma-kmalloc-256 (Not tainted): Redzone overwritten
[    2.516907] -----------------------------------------------------------------------------
[    2.516907]
[    2.526535] Disabling lock debugging due to kernel taint
[    2.531845] INFO: 0xde858108-0xde85810b. First byte 0xf8 instead of 0xcc
[    2.538549] INFO: Allocated in 0x806181a9 age=0 cpu=0 pid=58
[    2.544229] 	__kmalloc+0x374/0x564
[    2.547649] 	talitos_edesc_alloc+0x17c/0x48c
[    2.551929] 	aead_edesc_alloc+0x80/0x154
[    2.555863] 	aead_encrypt+0x30/0xe0
[    2.559368] 	__test_aead+0x5a0/0x1f3c
[    2.563042] 	test_aead+0x2c/0x110
[    2.566371] 	alg_test_aead+0x5c/0xf4
[    2.569958] 	alg_test+0x1dc/0x5a0
[    2.573305] 	cryptomgr_test+0x50/0x70
[    2.576984] 	kthread+0xd8/0x134
[    2.580155] 	ret_from_kernel_thread+0x5c/0x64
[    2.584534] INFO: Freed in ipsec_esp_encrypt_done+0x130/0x240 age=6 cpu=0 pid=0
[    2.591839] 	ipsec_esp_encrypt_done+0x130/0x240
[    2.596395] 	flush_channel+0x1dc/0x488
[    2.600161] 	talitos2_done_4ch+0x30/0x200
[    2.604185] 	tasklet_action+0xa0/0x13c
[    2.607948] 	__do_softirq+0x148/0x6cc
[    2.611623] 	irq_exit+0xc0/0x124
[    2.614869] 	call_do_irq+0x24/0x3c
[    2.618292] 	do_IRQ+0x78/0x108
[    2.621369] 	ret_from_except+0x0/0x14
[    2.625055] 	finish_task_switch+0x58/0x350
[    2.629165] 	schedule+0x80/0x134
[    2.632409] 	schedule_preempt_disabled+0x38/0xc8
[    2.637042] 	cpu_startup_entry+0xe4/0x190
[    2.641074] 	start_kernel+0x3f4/0x408
[    2.644741] 	0x3438
[    2.646857] INFO: Slab 0xdffbdb00 objects=9 used=1 fp=0xde8581c0 flags=0x0080
[    2.653978] INFO: Object 0xde858008 @offset=8 fp=0xca4395df
[    2.653978]
[    2.661032] Redzone de858000: cc cc cc cc cc cc cc cc                          ........
[    2.669029] Object de858008: 00 00 00 02 00 00 00 02 00 6b 6b 6b 1e 83 ea 28  .........kkk...(
[    2.677628] Object de858018: 00 00 00 70 1e 85 80 64 ff 73 1d 21 6b 6b 6b 6b  ...p...d.s.!kkkk
[    2.686228] Object de858028: 00 20 00 00 1e 84 17 24 00 10 00 00 1e 85 70 00  . .....$......p.
[    2.694829] Object de858038: 00 18 00 00 1e 84 17 44 00 08 00 00 1e 83 ea 28  .......D.......(
[    2.703430] Object de858048: 00 80 00 00 1e 84 f0 00 00 80 00 00 1e 85 70 10  ..............p.
[    2.712030] Object de858058: 00 20 6b 00 1e 85 80 f4 6b 6b 6b 6b 00 80 02 00  . k.....kkkk....
[    2.720629] Object de858068: 1e 84 f0 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  ....kkkkkkkkkkkk
[    2.729230] Object de858078: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[    2.737830] Object de858088: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[    2.746429] Object de858098: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[    2.755029] Object de8580a8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[    2.763628] Object de8580b8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[    2.772229] Object de8580c8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[    2.780829] Object de8580d8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[    2.789430] Object de8580e8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 73 b0 ea 9f  kkkkkkkkkkkks...
[    2.798030] Object de8580f8: e8 18 80 d6 56 38 44 c0 db e3 4f 71 f7 ce d1 d3  ....V8D...Oq....
[    2.806629] Redzone de858108: f8 bd 3e 4f                                      ..>O
[    2.814279] Padding de8581b0: 5a 5a 5a 5a 5a 5a 5a 5a                          ZZZZZZZZ
[    2.822283] CPU: 0 PID: 0 Comm: swapper Tainted: G    B           4.9.50-g995be12679 #179
[    2.831819] Call Trace:
[    2.834301] [dffefd20] [c01aa9a8] check_bytes_and_report+0x100/0x194 (unreliable)
[    2.841801] [dffefd50] [c01aac3c] check_object+0x200/0x530
[    2.847306] [dffefd80] [c01ae584] free_debug_processing+0x290/0x690
[    2.853585] [dffefde0] [c01aec8c] __slab_free+0x308/0x628
[    2.859000] [dffefe80] [c05057f4] ipsec_esp_encrypt_done+0x130/0x240
[    2.865378] [dffefeb0] [c05002c4] flush_channel+0x1dc/0x488
[    2.870968] [dffeff10] [c05007a8] talitos2_done_4ch+0x30/0x200
[    2.876814] [dffeff30] [c002fe38] tasklet_action+0xa0/0x13c
[    2.882399] [dffeff60] [c002f118] __do_softirq+0x148/0x6cc
[    2.887896] [dffeffd0] [c002f954] irq_exit+0xc0/0x124
[    2.892968] [dffefff0] [c0013adc] call_do_irq+0x24/0x3c
[    2.898213] [c0d4be00] [c000757c] do_IRQ+0x78/0x108
[    2.903113] [c0d4be30] [c0015c08] ret_from_except+0x0/0x14
[    2.908634] --- interrupt: 501 at finish_task_switch+0x70/0x350
[    2.908634]     LR = finish_task_switch+0x58/0x350
[    2.919327] [c0d4bf20] [c085e1d4] schedule+0x80/0x134
[    2.924398] [c0d4bf50] [c085e2c0] schedule_preempt_disabled+0x38/0xc8
[    2.930853] [c0d4bf60] [c007f064] cpu_startup_entry+0xe4/0x190
[    2.936707] [c0d4bfb0] [c096c434] start_kernel+0x3f4/0x408
[    2.942198] [c0d4bff0] [00003438] 0x3438
[    2.946137] FIX dma-kmalloc-256: Restoring 0xde858108-0xde85810b=0xcc
[    2.946137]
[    2.954158] FIX dma-kmalloc-256: Object at 0xde858008 not freed

This patch reworks the handling of the CICV out in order
to properly handle all cases.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/crypto/talitos.c | 40 ++++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index a5b608b54c74..e7e31f8fd3d1 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1239,14 +1239,15 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
 			dma_map_sg(dev, areq->dst, sg_count, DMA_FROM_DEVICE);
 	}
 
-	sg_count = talitos_sg_map(dev, areq->dst, cryptlen, edesc,
-				  &desc->ptr[5], sg_count, areq->assoclen,
-				  tbl_off);
+	ret = talitos_sg_map(dev, areq->dst, cryptlen, edesc, &desc->ptr[5],
+			     sg_count, areq->assoclen, tbl_off);
 
 	if (desc->hdr & DESC_HDR_TYPE_IPSEC_ESP)
 		to_talitos_ptr_ext_or(&desc->ptr[5], authsize, is_sec1);
 
-	if (sg_count > 1) {
+	/* ICV data */
+	if (ret > 1) {
+		tbl_off += ret;
 		edesc->icv_ool = true;
 		sync_needed = true;
 
@@ -1256,9 +1257,7 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
 				     sizeof(struct talitos_ptr) + authsize;
 
 			/* Add an entry to the link table for ICV data */
-			tbl_ptr += sg_count - 1;
-			to_talitos_ptr_ext_set(tbl_ptr, 0, is_sec1);
-			tbl_ptr++;
+			to_talitos_ptr_ext_set(tbl_ptr - 1, 0, is_sec1);
 			to_talitos_ptr_ext_set(tbl_ptr, DESC_PTR_LNKTBL_RETURN,
 					       is_sec1);
 			to_talitos_ptr_len(tbl_ptr, authsize, is_sec1);
@@ -1266,14 +1265,27 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
 			/* icv data follows link tables */
 			to_talitos_ptr(tbl_ptr, edesc->dma_link_tbl + offset,
 				       is_sec1);
-		}
-	}
+		} else {
+			dma_addr_t addr = edesc->dma_link_tbl;
 
-	/* ICV data */
-	if (!(desc->hdr & DESC_HDR_TYPE_IPSEC_ESP)) {
-		to_talitos_ptr_len(&desc->ptr[6], authsize, is_sec1);
-		to_talitos_ptr(&desc->ptr[6], edesc->dma_link_tbl +
-			       areq->assoclen + cryptlen, is_sec1);
+			if (is_sec1)
+				addr += areq->assoclen + cryptlen;
+			else
+				addr += sizeof(struct talitos_ptr) * tbl_off;
+
+			to_talitos_ptr(&desc->ptr[6], addr, is_sec1);
+			to_talitos_ptr_len(&desc->ptr[6], authsize, is_sec1);
+		}
+	} else if (!(desc->hdr & DESC_HDR_TYPE_IPSEC_ESP)) {
+		ret = talitos_sg_map(dev, areq->dst, authsize, edesc,
+				     &desc->ptr[6], sg_count, areq->assoclen +
+							      cryptlen,
+				     tbl_off);
+		if (ret > 1) {
+			tbl_off += ret;
+			edesc->icv_ool = true;
+			sync_needed = true;
+		}
 	}
 
 	/* iv out */
-- 
2.13.3

Re: [PATCH 5/6] crypto: talitos - use kzalloc instead of kmalloc

[Christophe LEROY]

Le 21/09/2017 à 09:19, Christophe Leroy a écrit :
> Use kzalloc() to zeroize the extended descriptor at allocation and
> further zeorising

This patch significantly degrades performances.
I will submit a new serie soon.

Christophe

> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>   drivers/crypto/talitos.c | 23 +----------------------
>   drivers/crypto/talitos.h |  2 --
>   2 files changed, 1 insertion(+), 24 deletions(-)
> 
> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> index cd8a37e60259..a5b608b54c74 100644
> --- a/drivers/crypto/talitos.c
> +++ b/drivers/crypto/talitos.c
> @@ -75,7 +75,6 @@ static void to_talitos_ptr_len(struct talitos_ptr *ptr, unsigned int len,
>   			       bool is_sec1)
>   {
>   	if (is_sec1) {
> -		ptr->res = 0;
>   		ptr->len1 = cpu_to_be16(len);
>   	} else {
>   		ptr->len = cpu_to_be16(len);
> @@ -118,7 +117,6 @@ static void map_single_talitos_ptr(struct device *dev,
>   
>   	to_talitos_ptr_len(ptr, len, is_sec1);
>   	to_talitos_ptr(ptr, dma_addr, is_sec1);
> -	to_talitos_ptr_ext_set(ptr, 0, is_sec1);
>   }
>   
>   /*
> @@ -287,7 +285,6 @@ int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,
>   	/* map descriptor and save caller data */
>   	if (is_sec1) {
>   		desc->hdr1 = desc->hdr;
> -		desc->next_desc = 0;
>   		request->dma_desc = dma_map_single(dev, &desc->hdr1,
>   						   TALITOS_DESC_SIZE,
>   						   DMA_BIDIRECTIONAL);
> @@ -1099,7 +1096,6 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
>   		to_talitos_ptr(link_tbl_ptr + count,
>   			       sg_dma_address(sg) + offset, 0);
>   		to_talitos_ptr_len(link_tbl_ptr + count, len, 0);
> -		to_talitos_ptr_ext_set(link_tbl_ptr + count, 0, 0);
>   		count++;
>   		cryptlen -= len;
>   		offset = 0;
> @@ -1125,7 +1121,6 @@ int talitos_sg_map(struct device *dev, struct scatterlist *src,
>   	bool is_sec1 = has_ftr_sec1(priv);
>   
>   	to_talitos_ptr_len(ptr, len, is_sec1);
> -	to_talitos_ptr_ext_set(ptr, 0, is_sec1);
>   
>   	if (sg_count == 1) {
>   		to_talitos_ptr(ptr, sg_dma_address(src) + offset, is_sec1);
> @@ -1197,11 +1192,9 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
>   	if (desc->hdr & DESC_HDR_TYPE_IPSEC_ESP) {
>   		to_talitos_ptr(&desc->ptr[2], edesc->iv_dma, is_sec1);
>   		to_talitos_ptr_len(&desc->ptr[2], ivsize, is_sec1);
> -		to_talitos_ptr_ext_set(&desc->ptr[2], 0, is_sec1);
>   	} else {
>   		to_talitos_ptr(&desc->ptr[3], edesc->iv_dma, is_sec1);
>   		to_talitos_ptr_len(&desc->ptr[3], ivsize, is_sec1);
> -		to_talitos_ptr_ext_set(&desc->ptr[3], 0, is_sec1);
>   	}
>   
>   	/* cipher key */
> @@ -1221,7 +1214,6 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
>   	 * typically 12 for ipsec
>   	 */
>   	to_talitos_ptr_len(&desc->ptr[4], cryptlen, is_sec1);
> -	to_talitos_ptr_ext_set(&desc->ptr[4], 0, is_sec1);
>   
>   	sg_link_tbl_len = cryptlen;
>   
> @@ -1275,8 +1267,6 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
>   			to_talitos_ptr(tbl_ptr, edesc->dma_link_tbl + offset,
>   				       is_sec1);
>   		}
> -	} else {
> -		edesc->icv_ool = false;
>   	}
>   
>   	/* ICV data */
> @@ -1386,7 +1376,7 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
>   		alloc_len += icv_stashing ? authsize : 0;
>   	}
>   
> -	edesc = kmalloc(alloc_len, GFP_DMA | flags);
> +	edesc = kzalloc(alloc_len, GFP_DMA | flags);
>   	if (!edesc) {
>   		dev_err(dev, "could not allocate edescriptor\n");
>   		err = ERR_PTR(-ENOMEM);
> @@ -1467,7 +1457,6 @@ static int aead_decrypt(struct aead_request *req)
>   				  DESC_HDR_MODE1_MDEU_CICV;
>   
>   		/* reset integrity check result bits */
> -		edesc->desc.hdr_lo = 0;
>   
>   		return ipsec_esp(edesc, req, ipsec_esp_decrypt_hwauth_done);
>   	}
> @@ -1554,12 +1543,10 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
>   	bool is_sec1 = has_ftr_sec1(priv);
>   
>   	/* first DWORD empty */
> -	desc->ptr[0] = zero_entry;
>   
>   	/* cipher iv */
>   	to_talitos_ptr(&desc->ptr[1], edesc->iv_dma, is_sec1);
>   	to_talitos_ptr_len(&desc->ptr[1], ivsize, is_sec1);
> -	to_talitos_ptr_ext_set(&desc->ptr[1], 0, is_sec1);
>   
>   	/* cipher key */
>   	map_single_talitos_ptr(dev, &desc->ptr[2], ctx->keylen,
> @@ -1598,7 +1585,6 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
>   			       DMA_FROM_DEVICE);
>   
>   	/* last DWORD empty */
> -	desc->ptr[6] = zero_entry;
>   
>   	if (sync_needed)
>   		dma_sync_single_for_device(dev, edesc->dma_link_tbl,
> @@ -1744,7 +1730,6 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
>   	int sg_count;
>   
>   	/* first DWORD empty */
> -	desc->ptr[0] = zero_entry;
>   
>   	/* hash context in */
>   	if (!req_ctx->first || req_ctx->swinit) {
> @@ -1753,8 +1738,6 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
>   				       (char *)req_ctx->hw_context,
>   				       DMA_TO_DEVICE);
>   		req_ctx->swinit = 0;
> -	} else {
> -		desc->ptr[1] = zero_entry;
>   	}
>   	/* Indicate next op is not the first. */
>   	req_ctx->first = 0;
> @@ -1763,8 +1746,6 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
>   	if (ctx->keylen)
>   		map_single_talitos_ptr(dev, &desc->ptr[2], ctx->keylen,
>   				       (char *)&ctx->key, DMA_TO_DEVICE);
> -	else
> -		desc->ptr[2] = zero_entry;
>   
>   	sg_count = edesc->src_nents ?: 1;
>   	if (is_sec1 && sg_count > 1)
> @@ -1781,7 +1762,6 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
>   		sync_needed = true;
>   
>   	/* fifth DWORD empty */
> -	desc->ptr[4] = zero_entry;
>   
>   	/* hash/HMAC out -or- hash context out */
>   	if (req_ctx->last)
> @@ -1794,7 +1774,6 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
>   				       req_ctx->hw_context, DMA_FROM_DEVICE);
>   
>   	/* last DWORD empty */
> -	desc->ptr[6] = zero_entry;
>   
>   	if (is_sec1 && from_talitos_ptr_len(&desc->ptr[3], true) == 0)
>   		talitos_handle_buggy_hash(ctx, edesc, &desc->ptr[3]);
> diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
> index 8dd8f40e2771..6112ff1fc334 100644
> --- a/drivers/crypto/talitos.h
> +++ b/drivers/crypto/talitos.h
> @@ -52,8 +52,6 @@ struct talitos_ptr {
>   	__be32 ptr;     /* address */
>   };
>   
> -static const struct talitos_ptr zero_entry;
> -
>   /* descriptor */
>   struct talitos_desc {
>   	__be32 hdr;                     /* header high bits */
> 

Re: [PATCH 0/6] crypto: talitos - various fixes

[christophe leroy]

What's going wrong ? I have resent it and it the same again, the serie 
is not dispatched on the linux-crypto list, but when I answer to a mail 
of the serie, the answer shows up.

What could be the issue ?

Christophe

Le 20/09/2017 à 10:35, Christophe LEROY a écrit :
> Looks like the linux-crypto list and patchwork was out of order when I 
> sent this. I will send again, sorry for the noise on the other lists.
> 
> Le 19/09/2017 à 14:58, Christophe Leroy a écrit :
>> This serie provide various fixes on the talitos driver.
>>
>> Christophe Leroy (6):
>>    crypto: talitos - Don't provide setkey for non hmac hashing algs.
>>    crypto: talitos - fix hashing
>>    crypto: talitos - fix sha224
>>    crypto: talitos - fix AEAD test failures
>>    crypto: talitos - use kzalloc instead of kmalloc
>>    crypto: talitos - fix memory corruption on SEC2
>>
>>   drivers/crypto/talitos.c | 81 
>> +++++++++++++++++++++---------------------------
>>   drivers/crypto/talitos.h |  2 --
>>   2 files changed, 36 insertions(+), 47 deletions(-)
>>

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH 0/6] crypto: talitos - various fixes

[Christophe LEROY]

Looks like the linux-crypto list and patchwork was out of order when I 
sent this. I will send again, sorry for the noise on the other lists.

Le 19/09/2017 à 14:58, Christophe Leroy a écrit :
> This serie provide various fixes on the talitos driver.
> 
> Christophe Leroy (6):
>    crypto: talitos - Don't provide setkey for non hmac hashing algs.
>    crypto: talitos - fix hashing
>    crypto: talitos - fix sha224
>    crypto: talitos - fix AEAD test failures
>    crypto: talitos - use kzalloc instead of kmalloc
>    crypto: talitos - fix memory corruption on SEC2
> 
>   drivers/crypto/talitos.c | 81 +++++++++++++++++++++---------------------------
>   drivers/crypto/talitos.h |  2 --
>   2 files changed, 36 insertions(+), 47 deletions(-)
> 

[PATCH 0/6] crypto: talitos - various fixes

[Christophe Leroy]

This serie provide various fixes on the talitos driver.

Christophe Leroy (6):
  crypto: talitos - Don't provide setkey for non hmac hashing algs.
  crypto: talitos - fix hashing
  crypto: talitos - fix sha224
  crypto: talitos - fix AEAD test failures
  crypto: talitos - use kzalloc instead of kmalloc
  crypto: talitos - fix memory corruption on SEC2

 drivers/crypto/talitos.c | 81 +++++++++++++++++++++---------------------------
 drivers/crypto/talitos.h |  2 --
 2 files changed, 36 insertions(+), 47 deletions(-)

-- 
2.13.3

[PATCH 0/6] crypto: talitos - various fixes

[Christophe Leroy]

This serie provide various fixes on the talitos driver.

Christophe Leroy (6):
  crypto: talitos - Don't provide setkey for non hmac hashing algs.
  crypto: talitos - fix hashing
  crypto: talitos - fix sha224
  crypto: talitos - fix AEAD test failures
  crypto: talitos - use kzalloc instead of kmalloc
  crypto: talitos - fix memory corruption on SEC2

 drivers/crypto/talitos.c | 81 +++++++++++++++++++++---------------------------
 drivers/crypto/talitos.h |  2 --
 2 files changed, 36 insertions(+), 47 deletions(-)

-- 
2.13.3