2019-05-21 13:34:55

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 00/15] Fixing selftests failure on Talitos driver

Several test failures have popped up following recent changes to crypto
selftests.

This series fixes (most of) them.

The last three patches are trivial cleanups.

Christophe Leroy (15):
crypto: talitos - fix skcipher failure due to wrong output IV
crypto: talitos - rename alternative AEAD algos.
crypto: talitos - reduce max key size for SEC1
crypto: talitos - check AES key size
crypto: talitos - fix CTR alg blocksize
crypto: talitos - check data blocksize in ablkcipher.
crypto: talitos - fix ECB algs ivsize
crypto: talitos - Do not modify req->cryptlen on decryption.
crypto: talitos - HMAC SNOOP NO AFEU mode requires SW icv checking.
crypto: talitos - properly handle split ICV.
crypto: talitos - Align SEC1 accesses to 32 bits boundaries.
crypto: talitos - fix AEAD processing.
Revert "crypto: talitos - export the talitos_submit function"
crypto: talitos - use IS_ENABLED() in has_ftr_sec1()
crypto: talitos - use SPDX-License-Identifier

drivers/crypto/talitos.c | 281 ++++++++++++++++++++++-------------------------
drivers/crypto/talitos.h | 45 ++------
2 files changed, 139 insertions(+), 187 deletions(-)

--
2.13.3



2019-05-21 13:34:56

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 05/15] crypto: talitos - fix CTR alg blocksize

CTR has a blocksize of 1.

Signed-off-by: Christophe Leroy <[email protected]>
Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
---
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 95f71e18bf55..8b9a529f1b66 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2822,7 +2822,7 @@ static struct talitos_alg_template driver_algs[] = {
.alg.crypto = {
.cra_name = "ctr(aes)",
.cra_driver_name = "ctr-aes-talitos",
- .cra_blocksize = AES_BLOCK_SIZE,
+ .cra_blocksize = 1,
.cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER |
CRYPTO_ALG_ASYNC,
.cra_ablkcipher = {
--
2.13.3


2019-05-21 13:35:20

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 14/15] crypto: talitos - use IS_ENABLED() in has_ftr_sec1()

This patch rewrites has_ftr_sec1() using IS_ENABLED()
instead of #ifdefs

Signed-off-by: Christophe Leroy <[email protected]>
---
drivers/crypto/talitos.h | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index 95e97951b924..5699d46401e6 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -164,13 +164,11 @@ struct talitos_private {
*/
static inline bool has_ftr_sec1(struct talitos_private *priv)
{
-#if defined(CONFIG_CRYPTO_DEV_TALITOS1) && defined(CONFIG_CRYPTO_DEV_TALITOS2)
- return priv->features & TALITOS_FTR_SEC1 ? true : false;
-#elif defined(CONFIG_CRYPTO_DEV_TALITOS1)
- return true;
-#else
- return false;
-#endif
+ if (IS_ENABLED(CONFIG_CRYPTO_DEV_TALITOS1) &&
+ IS_ENABLED(CONFIG_CRYPTO_DEV_TALITOS2))
+ return priv->features & TALITOS_FTR_SEC1;
+
+ return IS_ENABLED(CONFIG_CRYPTO_DEV_TALITOS1);
}

/*
--
2.13.3


2019-05-21 13:35:29

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 13/15] Revert "crypto: talitos - export the talitos_submit function"

There is no other file using talitos_submit in the kernel tree,
so it doesn't need to be exported nor made global.

This reverts commit 865d506155b117edc7e668ced373030ce7108ce9.

Signed-off-by: Christophe Leroy <[email protected]>
Fixes: 865d506155b1 ("crypto: talitos - export the talitos_submit function")
---
drivers/crypto/talitos.c | 11 +++++------
drivers/crypto/talitos.h | 6 ------
2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 12917fd54bcb..6fe900d8e5d8 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -278,11 +278,11 @@ static int init_device(struct device *dev)
* callback must check err and feedback in descriptor header
* for device processing status.
*/
-int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,
- void (*callback)(struct device *dev,
- struct talitos_desc *desc,
- void *context, int error),
- void *context)
+static int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,
+ void (*callback)(struct device *dev,
+ struct talitos_desc *desc,
+ void *context, int error),
+ void *context)
{
struct talitos_private *priv = dev_get_drvdata(dev);
struct talitos_request *request;
@@ -332,7 +332,6 @@ int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,

return -EINPROGRESS;
}
-EXPORT_SYMBOL(talitos_submit);

/*
* process what was done, notify callback of error if not
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index dbedd0956c8a..95e97951b924 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -150,12 +150,6 @@ struct talitos_private {
bool rng_registered;
};

-extern int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,
- void (*callback)(struct device *dev,
- struct talitos_desc *desc,
- void *context, int error),
- void *context);
-
/* .features flag */
#define TALITOS_FTR_SRC_LINK_TBL_LEN_INCLUDES_EXTENT 0x00000001
#define TALITOS_FTR_HW_AUTH_CHECK 0x00000002
--
2.13.3


2019-05-21 13:35:31

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 02/15] crypto: talitos - rename alternative AEAD algos.

The talitos driver has two ways to perform AEAD depending on the
HW capability. Some HW support both. It is needed to give them
different names to distingish which one it is for instance when
a test fails.

Signed-off-by: Christophe Leroy <[email protected]>
Fixes: 7405c8d7ff97 ("crypto: talitos - templates for AEAD using HMAC_SNOOP_NO_AFEU")
Cc: [email protected]
---
drivers/crypto/talitos.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index f443cbe7da80..6f8bc6467706 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2356,7 +2356,7 @@ static struct talitos_alg_template driver_algs[] = {
.base = {
.cra_name = "authenc(hmac(sha1),cbc(aes))",
.cra_driver_name = "authenc-hmac-sha1-"
- "cbc-aes-talitos",
+ "cbc-aes-talitos-hsna",
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2401,7 +2401,7 @@ static struct talitos_alg_template driver_algs[] = {
.cra_name = "authenc(hmac(sha1),"
"cbc(des3_ede))",
.cra_driver_name = "authenc-hmac-sha1-"
- "cbc-3des-talitos",
+ "cbc-3des-talitos-hsna",
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2444,7 +2444,7 @@ static struct talitos_alg_template driver_algs[] = {
.base = {
.cra_name = "authenc(hmac(sha224),cbc(aes))",
.cra_driver_name = "authenc-hmac-sha224-"
- "cbc-aes-talitos",
+ "cbc-aes-talitos-hsna",
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2489,7 +2489,7 @@ static struct talitos_alg_template driver_algs[] = {
.cra_name = "authenc(hmac(sha224),"
"cbc(des3_ede))",
.cra_driver_name = "authenc-hmac-sha224-"
- "cbc-3des-talitos",
+ "cbc-3des-talitos-hsna",
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2532,7 +2532,7 @@ static struct talitos_alg_template driver_algs[] = {
.base = {
.cra_name = "authenc(hmac(sha256),cbc(aes))",
.cra_driver_name = "authenc-hmac-sha256-"
- "cbc-aes-talitos",
+ "cbc-aes-talitos-hsna",
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2577,7 +2577,7 @@ static struct talitos_alg_template driver_algs[] = {
.cra_name = "authenc(hmac(sha256),"
"cbc(des3_ede))",
.cra_driver_name = "authenc-hmac-sha256-"
- "cbc-3des-talitos",
+ "cbc-3des-talitos-hsna",
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2706,7 +2706,7 @@ static struct talitos_alg_template driver_algs[] = {
.base = {
.cra_name = "authenc(hmac(md5),cbc(aes))",
.cra_driver_name = "authenc-hmac-md5-"
- "cbc-aes-talitos",
+ "cbc-aes-talitos-hsna",
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
@@ -2749,7 +2749,7 @@ static struct talitos_alg_template driver_algs[] = {
.base = {
.cra_name = "authenc(hmac(md5),cbc(des3_ede))",
.cra_driver_name = "authenc-hmac-md5-"
- "cbc-3des-talitos",
+ "cbc-3des-talitos-hsna",
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},
--
2.13.3


2019-05-21 13:36:03

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 10/15] crypto: talitos - properly handle split ICV.

The driver assumes that the ICV is as a single piece in the last
element of the scatterlist. This assumption is wrong.

This patch ensures that the ICV is properly handled regardless of
the scatterlist layout.

Fixes: 9c4a79653b35 ("crypto: talitos - Freescale integrated security engine (SEC) driver")
Signed-off-by: Christophe Leroy <[email protected]>
---
drivers/crypto/talitos.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index e35581d67315..7c8a3a717b91 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1069,7 +1069,6 @@ static void ipsec_esp_encrypt_done(struct device *dev,
unsigned int authsize = crypto_aead_authsize(authenc);
unsigned int ivsize = crypto_aead_ivsize(authenc);
struct talitos_edesc *edesc;
- struct scatterlist *sg;
void *icvdata;

edesc = container_of(desc, struct talitos_edesc, desc);
@@ -1083,9 +1082,8 @@ static void ipsec_esp_encrypt_done(struct device *dev,
else
icvdata = &edesc->link_tbl[edesc->src_nents +
edesc->dst_nents + 2];
- sg = sg_last(areq->dst, edesc->dst_nents);
- memcpy((char *)sg_virt(sg) + sg->length - authsize,
- icvdata, authsize);
+ sg_pcopy_from_buffer(areq->dst, edesc->dst_nents ? : 1, icvdata,
+ authsize, areq->assoclen + areq->cryptlen);
}

dma_unmap_single(dev, edesc->iv_dma, ivsize, DMA_TO_DEVICE);
@@ -1103,7 +1101,6 @@ static void ipsec_esp_decrypt_swauth_done(struct device *dev,
struct crypto_aead *authenc = crypto_aead_reqtfm(req);
unsigned int authsize = crypto_aead_authsize(authenc);
struct talitos_edesc *edesc;
- struct scatterlist *sg;
char *oicv, *icv;
struct talitos_private *priv = dev_get_drvdata(dev);
bool is_sec1 = has_ftr_sec1(priv);
@@ -1113,9 +1110,18 @@ static void ipsec_esp_decrypt_swauth_done(struct device *dev,
ipsec_esp_unmap(dev, edesc, req, false);

if (!err) {
+ char icvdata[SHA512_DIGEST_SIZE];
+ int nents = edesc->dst_nents ? : 1;
+ unsigned int len = req->assoclen + req->cryptlen;
+
/* auth check */
- sg = sg_last(req->dst, edesc->dst_nents ? : 1);
- icv = (char *)sg_virt(sg) + sg->length - authsize;
+ if (nents > 1) {
+ sg_pcopy_to_buffer(req->dst, nents, icvdata, authsize,
+ len - authsize);
+ icv = icvdata;
+ } else {
+ icv = (char *)sg_virt(req->dst) + len - authsize;
+ }

if (edesc->dma_len) {
if (is_sec1)
@@ -1537,7 +1543,6 @@ static int aead_decrypt(struct aead_request *req)
struct talitos_ctx *ctx = crypto_aead_ctx(authenc);
struct talitos_private *priv = dev_get_drvdata(ctx->dev);
struct talitos_edesc *edesc;
- struct scatterlist *sg;
void *icvdata;

/* allocate extended descriptor */
@@ -1571,9 +1576,8 @@ static int aead_decrypt(struct aead_request *req)
else
icvdata = &edesc->link_tbl[0];

- sg = sg_last(req->src, edesc->src_nents ? : 1);
-
- memcpy(icvdata, (char *)sg_virt(sg) + sg->length - authsize, authsize);
+ sg_pcopy_to_buffer(req->src, edesc->src_nents ? : 1, icvdata, authsize,
+ req->assoclen + req->cryptlen - authsize);

return ipsec_esp(edesc, req, false, ipsec_esp_decrypt_swauth_done);
}
--
2.13.3


2019-05-21 13:36:17

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 07/15] crypto: talitos - fix ECB algs ivsize

ECB's ivsize must be 0.

Signed-off-by: Christophe Leroy <[email protected]>
Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
---
drivers/crypto/talitos.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 1e5410f92166..6f6f34754ad8 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2809,7 +2809,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_ablkcipher = {
.min_keysize = AES_MIN_KEY_SIZE,
.max_keysize = AES_MAX_KEY_SIZE,
- .ivsize = AES_BLOCK_SIZE,
.setkey = ablkcipher_aes_setkey,
}
},
@@ -2862,7 +2861,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_ablkcipher = {
.min_keysize = DES_KEY_SIZE,
.max_keysize = DES_KEY_SIZE,
- .ivsize = DES_BLOCK_SIZE,
.setkey = ablkcipher_des_setkey,
}
},
@@ -2897,7 +2895,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_ablkcipher = {
.min_keysize = DES3_EDE_KEY_SIZE,
.max_keysize = DES3_EDE_KEY_SIZE,
- .ivsize = DES3_EDE_BLOCK_SIZE,
.setkey = ablkcipher_des3_setkey,
}
},
--
2.13.3


2019-05-21 13:36:19

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 15/15] crypto: talitos - use SPDX-License-Identifier

This patch drops the license text and replaces it
with an SPDX-License-Identifier tag.

Signed-off-by: Christophe Leroy <[email protected]>
---
drivers/crypto/talitos.c | 15 +--------------
drivers/crypto/talitos.h | 25 +------------------------
2 files changed, 2 insertions(+), 38 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 6fe900d8e5d8..32a7e747dc5f 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0+
/*
* talitos - Freescale Integrated Security Engine (SEC) device driver
*
@@ -9,20 +10,6 @@
* Crypto algorithm registration code copied from hifn driver:
* 2007+ Copyright (c) Evgeniy Polyakov <[email protected]>
* All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/

#include <linux/kernel.h>
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index 5699d46401e6..32ad4fc679ed 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -1,31 +1,8 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
/*
* Freescale SEC (talitos) device register and descriptor header defines
*
* Copyright (c) 2006-2011 Freescale Semiconductor, Inc.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. The name of the author may not be used to endorse or promote products
- * derived from this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
- * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
- * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
*/

#define TALITOS_TIMEOUT 100000
--
2.13.3


2019-05-21 13:36:57

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 12/15] crypto: talitos - fix AEAD processing.

This driver is working well in 'simple cases', but as soon as
more exotic SG lists are provided (dst different from src,
auth part not in a single SG fragment, ...) there are
wrong results, overruns, etc ...

This patch cleans up the AEAD processing by:
- Simplifying the location of 'out of line' ICV
- Never using 'out of line' ICV on encryp
- Always using 'out of line' ICV on decrypt
- Forcing the generation of a SG table on decrypt

Signed-off-by: Christophe Leroy <[email protected]>
Fixes: aeb4c132f33d ("crypto: talitos - Convert to new AEAD interface")
---
drivers/crypto/talitos.c | 158 ++++++++++++++++-------------------------------
drivers/crypto/talitos.h | 2 +-
2 files changed, 55 insertions(+), 105 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 750b0159e654..12917fd54bcb 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1040,8 +1040,8 @@ static void ipsec_esp_unmap(struct device *dev,
DMA_FROM_DEVICE);
unmap_single_talitos_ptr(dev, civ_ptr, DMA_TO_DEVICE);

- talitos_sg_unmap(dev, edesc, areq->src, areq->dst, cryptlen,
- areq->assoclen);
+ talitos_sg_unmap(dev, edesc, areq->src, areq->dst,
+ cryptlen + authsize, areq->assoclen);

if (edesc->dma_len)
dma_unmap_single(dev, edesc->dma_link_tbl, edesc->dma_len,
@@ -1062,30 +1062,15 @@ static void ipsec_esp_encrypt_done(struct device *dev,
struct talitos_desc *desc, void *context,
int err)
{
- struct talitos_private *priv = dev_get_drvdata(dev);
- bool is_sec1 = has_ftr_sec1(priv);
struct aead_request *areq = context;
struct crypto_aead *authenc = crypto_aead_reqtfm(areq);
- unsigned int authsize = crypto_aead_authsize(authenc);
unsigned int ivsize = crypto_aead_ivsize(authenc);
struct talitos_edesc *edesc;
- void *icvdata;

edesc = container_of(desc, struct talitos_edesc, desc);

ipsec_esp_unmap(dev, edesc, areq, true);

- /* copy the generated ICV to dst */
- if (edesc->icv_ool) {
- if (is_sec1)
- icvdata = edesc->buf + areq->assoclen + areq->cryptlen;
- else
- icvdata = &edesc->link_tbl[edesc->src_nents +
- edesc->dst_nents + 2];
- sg_pcopy_from_buffer(areq->dst, edesc->dst_nents ? : 1, icvdata,
- authsize, areq->assoclen + areq->cryptlen);
- }
-
dma_unmap_single(dev, edesc->iv_dma, ivsize, DMA_TO_DEVICE);

kfree(edesc);
@@ -1102,39 +1087,15 @@ static void ipsec_esp_decrypt_swauth_done(struct device *dev,
unsigned int authsize = crypto_aead_authsize(authenc);
struct talitos_edesc *edesc;
char *oicv, *icv;
- struct talitos_private *priv = dev_get_drvdata(dev);
- bool is_sec1 = has_ftr_sec1(priv);

edesc = container_of(desc, struct talitos_edesc, desc);

ipsec_esp_unmap(dev, edesc, req, false);

if (!err) {
- char icvdata[SHA512_DIGEST_SIZE];
- int nents = edesc->dst_nents ? : 1;
- unsigned int len = req->assoclen + req->cryptlen;
-
/* auth check */
- if (nents > 1) {
- sg_pcopy_to_buffer(req->dst, nents, icvdata, authsize,
- len - authsize);
- icv = icvdata;
- } else {
- icv = (char *)sg_virt(req->dst) + len - authsize;
- }
-
- if (edesc->dma_len) {
- if (is_sec1)
- oicv = (char *)&edesc->dma_link_tbl +
- req->assoclen + req->cryptlen;
- else
- oicv = (char *)
- &edesc->link_tbl[edesc->src_nents +
- edesc->dst_nents + 2];
- if (edesc->icv_ool)
- icv = oicv + authsize;
- } else
- oicv = (char *)&edesc->link_tbl[0];
+ oicv = edesc->buf + edesc->dma_len;
+ icv = oicv - authsize;

err = crypto_memneq(oicv, icv, authsize) ? -EBADMSG : 0;
}
@@ -1170,11 +1131,12 @@ static void ipsec_esp_decrypt_hwauth_done(struct device *dev,
* stop at cryptlen bytes
*/
static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
- unsigned int offset, int cryptlen,
+ unsigned int offset, int datalen, int elen,
struct talitos_ptr *link_tbl_ptr)
{
- int n_sg = sg_count;
+ int n_sg = elen ? sg_count + 1 : sg_count;
int count = 0;
+ int cryptlen = datalen + elen;

while (cryptlen && sg && n_sg--) {
unsigned int len = sg_dma_len(sg);
@@ -1189,11 +1151,20 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
if (len > cryptlen)
len = cryptlen;

+ if (datalen > 0 && len > datalen) {
+ to_talitos_ptr(link_tbl_ptr + count,
+ sg_dma_address(sg) + offset, datalen, 0);
+ to_talitos_ptr_ext_set(link_tbl_ptr + count, 0, 0);
+ count++;
+ len -= datalen;
+ offset += datalen;
+ }
to_talitos_ptr(link_tbl_ptr + count,
sg_dma_address(sg) + offset, len, 0);
to_talitos_ptr_ext_set(link_tbl_ptr + count, 0, 0);
count++;
cryptlen -= len;
+ datalen -= len;
offset = 0;

next:
@@ -1203,7 +1174,7 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
/* tag end of link table */
if (count > 0)
to_talitos_ptr_ext_set(link_tbl_ptr + count - 1,
- DESC_PTR_LNKTBL_RETURN, 0);
+ DESC_PTR_LNKTBL_RET, 0);

return count;
}
@@ -1211,7 +1182,8 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
static int talitos_sg_map_ext(struct device *dev, struct scatterlist *src,
unsigned int len, struct talitos_edesc *edesc,
struct talitos_ptr *ptr, int sg_count,
- unsigned int offset, int tbl_off, int elen)
+ unsigned int offset, int tbl_off, int elen,
+ bool force)
{
struct talitos_private *priv = dev_get_drvdata(dev);
bool is_sec1 = has_ftr_sec1(priv);
@@ -1221,7 +1193,7 @@ static int talitos_sg_map_ext(struct device *dev, struct scatterlist *src,
return 1;
}
to_talitos_ptr_ext_set(ptr, elen, is_sec1);
- if (sg_count == 1) {
+ if (sg_count == 1 && !force) {
to_talitos_ptr(ptr, sg_dma_address(src) + offset, len, is_sec1);
return sg_count;
}
@@ -1229,9 +1201,9 @@ static int talitos_sg_map_ext(struct device *dev, struct scatterlist *src,
to_talitos_ptr(ptr, edesc->dma_link_tbl + offset, len, is_sec1);
return sg_count;
}
- sg_count = sg_to_link_tbl_offset(src, sg_count, offset, len + elen,
+ sg_count = sg_to_link_tbl_offset(src, sg_count, offset, len, elen,
&edesc->link_tbl[tbl_off]);
- if (sg_count == 1) {
+ if (sg_count == 1 && !force) {
/* Only one segment now, so no link tbl needed*/
copy_talitos_ptr(ptr, &edesc->link_tbl[tbl_off], is_sec1);
return sg_count;
@@ -1249,7 +1221,7 @@ static int talitos_sg_map(struct device *dev, struct scatterlist *src,
unsigned int offset, int tbl_off)
{
return talitos_sg_map_ext(dev, src, len, edesc, ptr, sg_count, offset,
- tbl_off, 0);
+ tbl_off, 0, false);
}

/*
@@ -1277,6 +1249,7 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
bool is_ipsec_esp = desc->hdr & DESC_HDR_TYPE_IPSEC_ESP;
struct talitos_ptr *civ_ptr = &desc->ptr[is_ipsec_esp ? 2 : 3];
struct talitos_ptr *ckey_ptr = &desc->ptr[is_ipsec_esp ? 3 : 2];
+ dma_addr_t dma_icv = edesc->dma_link_tbl + edesc->dma_len - authsize;

/* hmac key */
to_talitos_ptr(&desc->ptr[0], ctx->dma_key, ctx->authkeylen, is_sec1);
@@ -1316,7 +1289,8 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
elen = authsize;

ret = talitos_sg_map_ext(dev, areq->src, cryptlen, edesc, &desc->ptr[4],
- sg_count, areq->assoclen, tbl_off, elen);
+ sg_count, areq->assoclen, tbl_off, elen,
+ false);

if (ret > 1) {
tbl_off += ret;
@@ -1330,55 +1304,35 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
dma_map_sg(dev, areq->dst, sg_count, DMA_FROM_DEVICE);
}

- ret = talitos_sg_map(dev, areq->dst, cryptlen, edesc, &desc->ptr[5],
- sg_count, areq->assoclen, tbl_off);
-
- if (is_ipsec_esp)
- to_talitos_ptr_ext_or(&desc->ptr[5], authsize, is_sec1);
+ if (is_ipsec_esp && encrypt)
+ elen = authsize;
+ else
+ elen = 0;
+ ret = talitos_sg_map_ext(dev, areq->dst, cryptlen, edesc, &desc->ptr[5],
+ sg_count, areq->assoclen, tbl_off, elen,
+ is_ipsec_esp && !encrypt);
+ tbl_off += ret;

/* ICV data */
- if (ret > 1) {
- tbl_off += ret;
- edesc->icv_ool = true;
- sync_needed = true;
+ edesc->icv_ool = !encrypt;

- if (is_ipsec_esp) {
- struct talitos_ptr *tbl_ptr = &edesc->link_tbl[tbl_off];
- int offset = (edesc->src_nents + edesc->dst_nents + 2) *
- sizeof(struct talitos_ptr) + authsize;
+ if (!encrypt && is_ipsec_esp) {
+ struct talitos_ptr *tbl_ptr = &edesc->link_tbl[tbl_off];

- /* Add an entry to the link table for ICV data */
- to_talitos_ptr_ext_set(tbl_ptr - 1, 0, is_sec1);
- to_talitos_ptr_ext_set(tbl_ptr, DESC_PTR_LNKTBL_RETURN,
- is_sec1);
+ /* Add an entry to the link table for ICV data */
+ to_talitos_ptr_ext_set(tbl_ptr - 1, 0, is_sec1);
+ to_talitos_ptr_ext_set(tbl_ptr, DESC_PTR_LNKTBL_RET, is_sec1);

- /* icv data follows link tables */
- to_talitos_ptr(tbl_ptr, edesc->dma_link_tbl + offset,
- authsize, is_sec1);
- } else {
- dma_addr_t addr = edesc->dma_link_tbl;
-
- if (is_sec1)
- addr += areq->assoclen + cryptlen;
- else
- addr += sizeof(struct talitos_ptr) * tbl_off;
-
- to_talitos_ptr(&desc->ptr[6], addr, authsize, is_sec1);
- }
+ /* icv data follows link tables */
+ to_talitos_ptr(tbl_ptr, dma_icv, authsize, is_sec1);
+ to_talitos_ptr_ext_or(&desc->ptr[5], authsize, is_sec1);
+ sync_needed = true;
+ } else if (!encrypt) {
+ to_talitos_ptr(&desc->ptr[6], dma_icv, authsize, is_sec1);
+ sync_needed = true;
} else if (!is_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;
- } else {
- edesc->icv_ool = false;
- }
- } else {
- edesc->icv_ool = false;
+ talitos_sg_map(dev, areq->dst, authsize, edesc, &desc->ptr[6],
+ sg_count, areq->assoclen + cryptlen, tbl_off);
}

/* iv out */
@@ -1461,18 +1415,18 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
* and space for two sets of ICVs (stashed and generated)
*/
alloc_len = sizeof(struct talitos_edesc);
- if (src_nents || dst_nents) {
+ if (src_nents || dst_nents || !encrypt) {
if (is_sec1)
dma_len = (src_nents ? src_len : 0) +
- (dst_nents ? dst_len : 0);
+ (dst_nents ? dst_len : 0) + authsize;
else
dma_len = (src_nents + dst_nents + 2) *
- sizeof(struct talitos_ptr) + authsize * 2;
+ sizeof(struct talitos_ptr) + authsize;
alloc_len += dma_len;
} else {
dma_len = 0;
- alloc_len += icv_stashing ? authsize : 0;
}
+ alloc_len += icv_stashing ? authsize : 0;

/* if its a ahash, add space for a second desc next to the first one */
if (is_sec1 && !dst)
@@ -1570,11 +1524,7 @@ static int aead_decrypt(struct aead_request *req)
edesc->desc.hdr = ctx->desc_hdr_template | DESC_HDR_DIR_INBOUND;

/* stash incoming ICV for later cmp with ICV generated by the h/w */
- if (edesc->dma_len)
- icvdata = (char *)&edesc->link_tbl[edesc->src_nents +
- edesc->dst_nents + 2];
- else
- icvdata = &edesc->link_tbl[0];
+ icvdata = edesc->buf + edesc->dma_len;

sg_pcopy_to_buffer(req->src, edesc->src_nents ? : 1, icvdata, authsize,
req->assoclen + req->cryptlen - authsize);
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index a65a63e0d6c1..dbedd0956c8a 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -412,5 +412,5 @@ static inline bool has_ftr_sec1(struct talitos_private *priv)

/* link table extent field bits */
#define DESC_PTR_LNKTBL_JUMP 0x80
-#define DESC_PTR_LNKTBL_RETURN 0x02
+#define DESC_PTR_LNKTBL_RET 0x02
#define DESC_PTR_LNKTBL_NEXT 0x01
--
2.13.3


2019-05-21 13:37:11

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v1 09/15] crypto: talitos - HMAC SNOOP NO AFEU mode requires SW icv checking.

In that mode, hardware ICV verification is not supported.

Signed-off-by: Christophe Leroy <[email protected]>
Fixes: 7405c8d7ff97 ("crypto: talitos - templates for AEAD using HMAC_SNOOP_NO_AFEU")
---
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 a15aa6d6ec33..e35581d67315 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1545,7 +1545,8 @@ static int aead_decrypt(struct aead_request *req)
if (IS_ERR(edesc))
return PTR_ERR(edesc);

- if ((priv->features & TALITOS_FTR_HW_AUTH_CHECK) &&
+ if ((edesc->desc.hdr & DESC_HDR_TYPE_IPSEC_ESP) &&
+ (priv->features & TALITOS_FTR_HW_AUTH_CHECK) &&
((!edesc->src_nents && !edesc->dst_nents) ||
priv->features & TALITOS_FTR_SRC_LINK_TBL_LEN_INCLUDES_EXTENT)) {

--
2.13.3


2019-05-21 17:57:16

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v1 02/15] crypto: talitos - rename alternative AEAD algos.

Hi Joe & Andy

On 05/21/2019 01:34 PM, Christophe Leroy wrote:
> The talitos driver has two ways to perform AEAD depending on the
> HW capability. Some HW support both. It is needed to give them
> different names to distingish which one it is for instance when
> a test fails.
>
> Signed-off-by: Christophe Leroy <[email protected]>
> Fixes: 7405c8d7ff97 ("crypto: talitos - templates for AEAD using HMAC_SNOOP_NO_AFEU")
> Cc: [email protected]
> ---
> drivers/crypto/talitos.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> index f443cbe7da80..6f8bc6467706 100644
> --- a/drivers/crypto/talitos.c
> +++ b/drivers/crypto/talitos.c
> @@ -2356,7 +2356,7 @@ static struct talitos_alg_template driver_algs[] = {
> .base = {
> .cra_name = "authenc(hmac(sha1),cbc(aes))",
> .cra_driver_name = "authenc-hmac-sha1-"
> - "cbc-aes-talitos",
> + "cbc-aes-talitos-hsna",

checkpatch reports the following warning on the above:

WARNING: quoted string split across lines
#27: FILE: drivers/crypto/talitos.c:2359:
.cra_driver_name = "authenc-hmac-sha1-"
+ "cbc-aes-talitos-hsna",


But when I fixes the patch as follows, I get another warning:

@@ -2355,8 +2355,7 @@ static struct talitos_alg_template driver_algs[] = {
.alg.aead = {
.base = {
.cra_name = "authenc(hmac(sha1),cbc(aes))",
- .cra_driver_name = "authenc-hmac-sha1-"
- "cbc-aes-talitos",
+ .cra_driver_name = "authenc-hmac-sha1-cbc-aes-talitos-hsna",
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_ASYNC,
},



WARNING: line over 80 characters
#28: FILE: drivers/crypto/talitos.c:2358:
+ .cra_driver_name = "authenc-hmac-sha1-cbc-aes-talitos-hsna",


So, how should this be fixed ?

Thanks
Christophe

> .cra_blocksize = AES_BLOCK_SIZE,
> .cra_flags = CRYPTO_ALG_ASYNC,
> },
> @@ -2401,7 +2401,7 @@ static struct talitos_alg_template driver_algs[] = {
> .cra_name = "authenc(hmac(sha1),"
> "cbc(des3_ede))",
> .cra_driver_name = "authenc-hmac-sha1-"
> - "cbc-3des-talitos",
> + "cbc-3des-talitos-hsna",
> .cra_blocksize = DES3_EDE_BLOCK_SIZE,
> .cra_flags = CRYPTO_ALG_ASYNC,
> },
> @@ -2444,7 +2444,7 @@ static struct talitos_alg_template driver_algs[] = {
> .base = {
> .cra_name = "authenc(hmac(sha224),cbc(aes))",
> .cra_driver_name = "authenc-hmac-sha224-"
> - "cbc-aes-talitos",
> + "cbc-aes-talitos-hsna",
> .cra_blocksize = AES_BLOCK_SIZE,
> .cra_flags = CRYPTO_ALG_ASYNC,
> },
> @@ -2489,7 +2489,7 @@ static struct talitos_alg_template driver_algs[] = {
> .cra_name = "authenc(hmac(sha224),"
> "cbc(des3_ede))",
> .cra_driver_name = "authenc-hmac-sha224-"
> - "cbc-3des-talitos",
> + "cbc-3des-talitos-hsna",
> .cra_blocksize = DES3_EDE_BLOCK_SIZE,
> .cra_flags = CRYPTO_ALG_ASYNC,
> },
> @@ -2532,7 +2532,7 @@ static struct talitos_alg_template driver_algs[] = {
> .base = {
> .cra_name = "authenc(hmac(sha256),cbc(aes))",
> .cra_driver_name = "authenc-hmac-sha256-"
> - "cbc-aes-talitos",
> + "cbc-aes-talitos-hsna",
> .cra_blocksize = AES_BLOCK_SIZE,
> .cra_flags = CRYPTO_ALG_ASYNC,
> },
> @@ -2577,7 +2577,7 @@ static struct talitos_alg_template driver_algs[] = {
> .cra_name = "authenc(hmac(sha256),"
> "cbc(des3_ede))",
> .cra_driver_name = "authenc-hmac-sha256-"
> - "cbc-3des-talitos",
> + "cbc-3des-talitos-hsna",
> .cra_blocksize = DES3_EDE_BLOCK_SIZE,
> .cra_flags = CRYPTO_ALG_ASYNC,
> },
> @@ -2706,7 +2706,7 @@ static struct talitos_alg_template driver_algs[] = {
> .base = {
> .cra_name = "authenc(hmac(md5),cbc(aes))",
> .cra_driver_name = "authenc-hmac-md5-"
> - "cbc-aes-talitos",
> + "cbc-aes-talitos-hsna",
> .cra_blocksize = AES_BLOCK_SIZE,
> .cra_flags = CRYPTO_ALG_ASYNC,
> },
> @@ -2749,7 +2749,7 @@ static struct talitos_alg_template driver_algs[] = {
> .base = {
> .cra_name = "authenc(hmac(md5),cbc(des3_ede))",
> .cra_driver_name = "authenc-hmac-md5-"
> - "cbc-3des-talitos",
> + "cbc-3des-talitos-hsna",
> .cra_blocksize = DES3_EDE_BLOCK_SIZE,
> .cra_flags = CRYPTO_ALG_ASYNC,
> },
>

2019-05-21 18:11:20

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v1 02/15] crypto: talitos - rename alternative AEAD algos.

On Tue, 2019-05-21 at 17:54 +0000, Christophe Leroy wrote:
> Hi Joe & Andy
[]
> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
[]
> > @@ -2356,7 +2356,7 @@ static struct talitos_alg_template driver_algs[] = {
> > .base = {
> > .cra_name = "authenc(hmac(sha1),cbc(aes))",
> > .cra_driver_name = "authenc-hmac-sha1-"
> > - "cbc-aes-talitos",
> > + "cbc-aes-talitos-hsna",
>
> checkpatch reports the following warning on the above:
>
> WARNING: quoted string split across lines
> #27: FILE: drivers/crypto/talitos.c:2359:
> .cra_driver_name = "authenc-hmac-sha1-"
> + "cbc-aes-talitos-hsna",
>
>
> But when I fixes the patch as follows, I get another warning:
>
> @@ -2355,8 +2355,7 @@ static struct talitos_alg_template driver_algs[] = {
> .alg.aead = {
> .base = {
> .cra_name = "authenc(hmac(sha1),cbc(aes))",
> - .cra_driver_name = "authenc-hmac-sha1-"
> - "cbc-aes-talitos",
> + .cra_driver_name = "authenc-hmac-sha1-cbc-aes-talitos-hsna",
> .cra_blocksize = AES_BLOCK_SIZE,
> .cra_flags = CRYPTO_ALG_ASYNC,
> },
>
>
>
> WARNING: line over 80 characters
> #28: FILE: drivers/crypto/talitos.c:2358:
> + .cra_driver_name = "authenc-hmac-sha1-cbc-aes-talitos-hsna",
>
>
> So, how should this be fixed ?

For at least now, by ignoring this checkpatch message.

It's a brainless script. You are not brainless.


2019-05-28 09:19:04

by Horia Geanta

[permalink] [raw]
Subject: Re: [PATCH v1 00/15] Fixing selftests failure on Talitos driver

On 5/21/2019 4:34 PM, Christophe Leroy wrote:
> Several test failures have popped up following recent changes to crypto
> selftests.
>
> This series fixes (most of) them.
>
> The last three patches are trivial cleanups.
>
Thanks Christophe.

For the series:
Reviewed-by: Horia Geant? <[email protected]>

Have you validated the changes also on SEC 2.x+?
Asking since IIRC you mentioned having only HW with SEC 1 and changes in patch
"crypto: talitos - fix AEAD processing." look quite complex.

Thanks,
Horia

2019-05-28 16:48:57

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v1 00/15] Fixing selftests failure on Talitos driver

Horia Geanta <[email protected]> a écrit :

> On 5/21/2019 4:34 PM, Christophe Leroy wrote:
>> Several test failures have popped up following recent changes to crypto
>> selftests.
>>
>> This series fixes (most of) them.
>>
>> The last three patches are trivial cleanups.
>>
> Thanks Christophe.
>
> For the series:
> Reviewed-by: Horia Geantă <[email protected]>
>
> Have you validated the changes also on SEC 2.x+?
> Asking since IIRC you mentioned having only HW with SEC 1 and
> changes in patch
> "crypto: talitos - fix AEAD processing." look quite complex.

When I ported the driver to SEC1 some years ago I only had a SEC 1.2
(mpc885) but I now have also a board with a mpc8321E which embeds a
SEC 2.2 so I also tested the changes on it.

Christophe

>
> Thanks,
> Horia


2019-05-30 13:42:33

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v1 00/15] Fixing selftests failure on Talitos driver

On Tue, May 21, 2019 at 01:34:06PM +0000, Christophe Leroy wrote:
> Several test failures have popped up following recent changes to crypto
> selftests.
>
> This series fixes (most of) them.
>
> The last three patches are trivial cleanups.
>
> Christophe Leroy (15):
> crypto: talitos - fix skcipher failure due to wrong output IV
> crypto: talitos - rename alternative AEAD algos.
> crypto: talitos - reduce max key size for SEC1
> crypto: talitos - check AES key size
> crypto: talitos - fix CTR alg blocksize
> crypto: talitos - check data blocksize in ablkcipher.
> crypto: talitos - fix ECB algs ivsize
> crypto: talitos - Do not modify req->cryptlen on decryption.
> crypto: talitos - HMAC SNOOP NO AFEU mode requires SW icv checking.
> crypto: talitos - properly handle split ICV.
> crypto: talitos - Align SEC1 accesses to 32 bits boundaries.
> crypto: talitos - fix AEAD processing.
> Revert "crypto: talitos - export the talitos_submit function"
> crypto: talitos - use IS_ENABLED() in has_ftr_sec1()
> crypto: talitos - use SPDX-License-Identifier
>
> drivers/crypto/talitos.c | 281 ++++++++++++++++++++++-------------------------
> drivers/crypto/talitos.h | 45 ++------
> 2 files changed, 139 insertions(+), 187 deletions(-)

Patch 1 was already applied.

2-15 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