Hello
This series is all fixes which I found on allwinner crypto drivers.
Regards
Corentin Labbe (19):
crypto: sun8i-ce: Fix minor style issue
crypto: sun8i-ce: do not allocate memory when handling requests
crypto: sun4i-ss: do not allocate backup IV on requests
crypto: sun8i-ss: rework handling of IV
crypto: sun8i-ss: handle zero sized sg
crypto: sun8i-ss: remove redundant test
crypto: sun8i-ss: test error before assigning
crypto: sun8i-ss: use sg_nents_for_len
crypto: sun8i-ss: do not allocate memory when handling hash requests
crypto: sun8i-ss: do not zeroize all pad
crypto: sun8i-ss: handle requests if last block is not modulo 64
crypto: sun8i-ss: rework debugging
crypto: sun8i-ss: Add function for handling hash padding
crypto: sun8i-ss: add hmac(sha1)
crypto: sun8i-ss: do not fallback if cryptlen is less than sg length
crypto: sun8i-ce: Add function for handling hash padding
crypto: sun8i-ce: use sg_nents_for_len
crypto: sun8i-ce: rework debugging
crypto: sun8i-ce: do not fallback if cryptlen is less than sg length
.../allwinner/sun4i-ss/sun4i-ss-cipher.c | 22 +-
drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h | 1 +
.../allwinner/sun8i-ce/sun8i-ce-cipher.c | 102 +++--
.../crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 54 ++-
.../crypto/allwinner/sun8i-ce/sun8i-ce-hash.c | 130 ++++--
drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h | 19 +-
.../allwinner/sun8i-ss/sun8i-ss-cipher.c | 180 +++++---
.../crypto/allwinner/sun8i-ss/sun8i-ss-core.c | 92 ++++-
.../crypto/allwinner/sun8i-ss/sun8i-ss-hash.c | 385 +++++++++++++++---
drivers/crypto/allwinner/sun8i-ss/sun8i-ss.h | 33 +-
10 files changed, 767 insertions(+), 251 deletions(-)
--
2.34.1
Instead of allocate memory on each requests, it is easier to
pre-allocate buffer for backup IV.
This made error path easier.
Signed-off-by: Corentin Labbe <[email protected]>
---
.../allwinner/sun4i-ss/sun4i-ss-cipher.c | 22 +++++++------------
drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h | 1 +
2 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
index 8dc2a475c601..a8c784acce13 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
@@ -20,7 +20,6 @@ static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)
unsigned int ivsize = crypto_skcipher_ivsize(tfm);
struct sun4i_cipher_req_ctx *ctx = skcipher_request_ctx(areq);
u32 mode = ctx->mode;
- void *backup_iv = NULL;
/* when activating SS, the default FIFO space is SS_RX_DEFAULT(32) */
u32 rx_cnt = SS_RX_DEFAULT;
u32 tx_cnt = 0;
@@ -47,10 +46,8 @@ static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)
}
if (areq->iv && ivsize > 0 && mode & SS_DECRYPTION) {
- backup_iv = kzalloc(ivsize, GFP_KERNEL);
- if (!backup_iv)
- return -ENOMEM;
- scatterwalk_map_and_copy(backup_iv, areq->src, areq->cryptlen - ivsize, ivsize, 0);
+ scatterwalk_map_and_copy(ctx->backup_iv, areq->src,
+ areq->cryptlen - ivsize, ivsize, 0);
}
if (IS_ENABLED(CONFIG_CRYPTO_DEV_SUN4I_SS_DEBUG)) {
@@ -133,8 +130,8 @@ static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)
if (areq->iv) {
if (mode & SS_DECRYPTION) {
- memcpy(areq->iv, backup_iv, ivsize);
- kfree_sensitive(backup_iv);
+ memcpy(areq->iv, ctx->backup_iv, ivsize);
+ memzero_explicit(ctx->backup_iv, ivsize);
} else {
scatterwalk_map_and_copy(areq->iv, areq->dst, areq->cryptlen - ivsize,
ivsize, 0);
@@ -198,7 +195,6 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
unsigned int ileft = areq->cryptlen;
unsigned int oleft = areq->cryptlen;
unsigned int todo;
- void *backup_iv = NULL;
struct sg_mapping_iter mi, mo;
unsigned long pi = 0, po = 0; /* progress for in and out */
bool miter_err;
@@ -242,10 +238,8 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
return sun4i_ss_cipher_poll_fallback(areq);
if (areq->iv && ivsize > 0 && mode & SS_DECRYPTION) {
- backup_iv = kzalloc(ivsize, GFP_KERNEL);
- if (!backup_iv)
- return -ENOMEM;
- scatterwalk_map_and_copy(backup_iv, areq->src, areq->cryptlen - ivsize, ivsize, 0);
+ scatterwalk_map_and_copy(ctx->backup_iv, areq->src,
+ areq->cryptlen - ivsize, ivsize, 0);
}
if (IS_ENABLED(CONFIG_CRYPTO_DEV_SUN4I_SS_DEBUG)) {
@@ -382,8 +376,8 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
}
if (areq->iv) {
if (mode & SS_DECRYPTION) {
- memcpy(areq->iv, backup_iv, ivsize);
- kfree_sensitive(backup_iv);
+ memcpy(areq->iv, ctx->backup_iv, ivsize);
+ memzero_explicit(ctx->backup_iv, ivsize);
} else {
scatterwalk_map_and_copy(areq->iv, areq->dst, areq->cryptlen - ivsize,
ivsize, 0);
diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h
index 0fee6f4e2d90..ba59c7a48825 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h
@@ -183,6 +183,7 @@ struct sun4i_tfm_ctx {
struct sun4i_cipher_req_ctx {
u32 mode;
+ u8 backup_iv[AES_BLOCK_SIZE];
struct skcipher_request fallback_req; // keep at the end
};
--
2.34.1
Some fallback tests were redundant with what sun8i_ss_hash_need_fallback() already do.
Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c
index ca4f280af35d..eaa0bbaf5581 100644
--- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c
+++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c
@@ -288,21 +288,11 @@ int sun8i_ss_hash_digest(struct ahash_request *areq)
struct sun8i_ss_alg_template *algt;
struct sun8i_ss_dev *ss;
struct crypto_engine *engine;
- struct scatterlist *sg;
- int nr_sgs, e, i;
+ int e;
if (sun8i_ss_hash_need_fallback(areq))
return sun8i_ss_hash_digest_fb(areq);
- nr_sgs = sg_nents(areq->src);
- if (nr_sgs > MAX_SG - 1)
- return sun8i_ss_hash_digest_fb(areq);
-
- for_each_sg(areq->src, sg, nr_sgs, i) {
- if (sg->length % 4 || !IS_ALIGNED(sg->offset, sizeof(u32)))
- return sun8i_ss_hash_digest_fb(areq);
- }
-
algt = container_of(alg, struct sun8i_ss_alg_template, alg.hash);
ss = algt->ss;
--
2.34.1
The "Fallback for xxx" message is annoying, remove it and store the
information in the debugfs.
In the same time, reports more fallback statistics.
Signed-off-by: Corentin Labbe <[email protected]>
---
.../allwinner/sun8i-ss/sun8i-ss-cipher.c | 41 ++++++++++++++-----
.../crypto/allwinner/sun8i-ss/sun8i-ss-core.c | 21 ++++++++++
.../crypto/allwinner/sun8i-ss/sun8i-ss-hash.c | 36 ++++++++++++----
drivers/crypto/allwinner/sun8i-ss/sun8i-ss.h | 7 +++-
4 files changed, 83 insertions(+), 22 deletions(-)
diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c
index c4cb1ab1eeaa..7f1940c6cc41 100644
--- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c
+++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c
@@ -22,35 +22,54 @@
static bool sun8i_ss_need_fallback(struct skcipher_request *areq)
{
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(areq);
+ struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
+ struct sun8i_ss_alg_template *algt = container_of(alg, struct sun8i_ss_alg_template, alg.skcipher);
struct scatterlist *in_sg = areq->src;
struct scatterlist *out_sg = areq->dst;
struct scatterlist *sg;
- if (areq->cryptlen == 0 || areq->cryptlen % 16)
+ if (areq->cryptlen == 0 || areq->cryptlen % 16) {
+ algt->stat_fb_len++;
return true;
+ }
if (sg_nents_for_len(areq->src, areq->cryptlen) > 8 ||
- sg_nents_for_len(areq->dst, areq->cryptlen) > 8)
+ sg_nents_for_len(areq->dst, areq->cryptlen) > 8) {
+ algt->stat_fb_sgnum++;
return true;
+ }
sg = areq->src;
while (sg) {
- if ((sg->length % 16) != 0)
+ if ((sg->length % 16) != 0) {
+ algt->stat_fb_sglen++;
return true;
- if ((sg_dma_len(sg) % 16) != 0)
+ }
+ if ((sg_dma_len(sg) % 16) != 0) {
+ algt->stat_fb_sglen++;
return true;
- if (!IS_ALIGNED(sg->offset, 16))
+ }
+ if (!IS_ALIGNED(sg->offset, 16)) {
+ algt->stat_fb_align++;
return true;
+ }
sg = sg_next(sg);
}
sg = areq->dst;
while (sg) {
- if ((sg->length % 16) != 0)
+ if ((sg->length % 16) != 0) {
+ algt->stat_fb_sglen++;
return true;
- if ((sg_dma_len(sg) % 16) != 0)
+ }
+ if ((sg_dma_len(sg) % 16) != 0) {
+ algt->stat_fb_sglen++;
return true;
- if (!IS_ALIGNED(sg->offset, 16))
+ }
+ if (!IS_ALIGNED(sg->offset, 16)) {
+ algt->stat_fb_align++;
return true;
+ }
sg = sg_next(sg);
}
@@ -385,9 +404,9 @@ int sun8i_ss_cipher_init(struct crypto_tfm *tfm)
crypto_skcipher_reqsize(op->fallback_tfm);
- dev_info(op->ss->dev, "Fallback for %s is %s\n",
- crypto_tfm_alg_driver_name(&sktfm->base),
- crypto_tfm_alg_driver_name(crypto_skcipher_tfm(op->fallback_tfm)));
+ memcpy(algt->fbname,
+ crypto_tfm_alg_driver_name(crypto_skcipher_tfm(op->fallback_tfm)),
+ CRYPTO_MAX_ALG_NAME);
op->enginectx.op.do_one_request = sun8i_ss_handle_cipher_request;
op->enginectx.op.prepare_request = NULL;
diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c
index 8d31fd4968f3..f09de5737e8b 100644
--- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c
+++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c
@@ -430,6 +430,17 @@ static int sun8i_ss_debugfs_show(struct seq_file *seq, void *v)
ss_algs[i].alg.skcipher.base.cra_driver_name,
ss_algs[i].alg.skcipher.base.cra_name,
ss_algs[i].stat_req, ss_algs[i].stat_fb);
+
+ seq_printf(seq, "\tLast fallback is: %s\n",
+ ss_algs[i].fbname);
+ seq_printf(seq, "\tFallback due to length: %lu\n",
+ ss_algs[i].stat_fb_len);
+ seq_printf(seq, "\tFallback due to SG length: %lu\n",
+ ss_algs[i].stat_fb_sglen);
+ seq_printf(seq, "\tFallback due to alignment: %lu\n",
+ ss_algs[i].stat_fb_align);
+ seq_printf(seq, "\tFallback due to SG numbers: %lu\n",
+ ss_algs[i].stat_fb_sgnum);
break;
case CRYPTO_ALG_TYPE_RNG:
seq_printf(seq, "%s %s reqs=%lu tsize=%lu\n",
@@ -442,6 +453,16 @@ static int sun8i_ss_debugfs_show(struct seq_file *seq, void *v)
ss_algs[i].alg.hash.halg.base.cra_driver_name,
ss_algs[i].alg.hash.halg.base.cra_name,
ss_algs[i].stat_req, ss_algs[i].stat_fb);
+ seq_printf(seq, "\tLast fallback is: %s\n",
+ ss_algs[i].fbname);
+ seq_printf(seq, "\tFallback due to length: %lu\n",
+ ss_algs[i].stat_fb_len);
+ seq_printf(seq, "\tFallback due to SG length: %lu\n",
+ ss_algs[i].stat_fb_sglen);
+ seq_printf(seq, "\tFallback due to alignment: %lu\n",
+ ss_algs[i].stat_fb_align);
+ seq_printf(seq, "\tFallback due to SG numbers: %lu\n",
+ ss_algs[i].stat_fb_sgnum);
break;
}
}
diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c
index 1b44c1a115d6..cb510ec21ec4 100644
--- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c
+++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c
@@ -51,9 +51,8 @@ int sun8i_ss_hash_crainit(struct crypto_tfm *tfm)
sizeof(struct sun8i_ss_hash_reqctx) +
crypto_ahash_reqsize(op->fallback_tfm));
- dev_info(op->ss->dev, "Fallback for %s is %s\n",
- crypto_tfm_alg_driver_name(tfm),
- crypto_tfm_alg_driver_name(&op->fallback_tfm->base));
+ memcpy(algt->fbname, crypto_tfm_alg_driver_name(&op->fallback_tfm->base), CRYPTO_MAX_ALG_NAME);
+
err = pm_runtime_get_sync(op->ss->dev);
if (err < 0)
goto error_pm;
@@ -259,16 +258,29 @@ static int sun8i_ss_run_hash_task(struct sun8i_ss_dev *ss,
static bool sun8i_ss_hash_need_fallback(struct ahash_request *areq)
{
+ struct crypto_ahash *tfm = crypto_ahash_reqtfm(areq);
+ struct ahash_alg *alg = __crypto_ahash_alg(tfm->base.__crt_alg);
+ struct sun8i_ss_alg_template *algt;
struct scatterlist *sg;
- if (areq->nbytes == 0)
+ algt = container_of(alg, struct sun8i_ss_alg_template, alg.hash);
+
+ if (areq->nbytes == 0) {
+ algt->stat_fb_len++;
return true;
- if (areq->nbytes >= MAX_PAD_SIZE - 64)
+ }
+
+ if (areq->nbytes >= MAX_PAD_SIZE - 64) {
+ algt->stat_fb_len++;
return true;
+ }
/* we need to reserve one SG for the padding one */
- if (sg_nents(areq->src) > MAX_SG - 1)
+ if (sg_nents(areq->src) > MAX_SG - 1) {
+ algt->stat_fb_sgnum++;
return true;
+ }
+
sg = areq->src;
while (sg) {
/* SS can operate hash only on full block size
@@ -276,12 +288,18 @@ static bool sun8i_ss_hash_need_fallback(struct ahash_request *areq)
* is always 64
*/
/* Only the last block could be bounced to the pad buffer */
- if (sg->length % 64 && sg_next(sg))
+ if (sg->length % 64 && sg_next(sg)) {
+ algt->stat_fb_sglen++;
return true;
- if (!IS_ALIGNED(sg->offset, sizeof(u32)))
+ }
+ if (!IS_ALIGNED(sg->offset, sizeof(u32))) {
+ algt->stat_fb_align++;
return true;
- if (sg->length % 4)
+ }
+ if (sg->length % 4) {
+ algt->stat_fb_sglen++;
return true;
+ }
sg = sg_next(sg);
}
return false;
diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss.h b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss.h
index 2e3524654aca..b56038de333b 100644
--- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss.h
+++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss.h
@@ -279,11 +279,14 @@ struct sun8i_ss_alg_template {
struct rng_alg rng;
struct ahash_alg hash;
} alg;
-#ifdef CONFIG_CRYPTO_DEV_SUN8I_SS_DEBUG
unsigned long stat_req;
unsigned long stat_fb;
unsigned long stat_bytes;
-#endif
+ unsigned long stat_fb_len;
+ unsigned long stat_fb_sglen;
+ unsigned long stat_fb_align;
+ unsigned long stat_fb_sgnum;
+ char fbname[CRYPTO_MAX_ALG_NAME];
};
int sun8i_ss_enqueue(struct crypto_async_request *areq, u32 type);
--
2.34.1
The first thing we should do after dma_map_single() is to test the
result.
Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c
index eaa0bbaf5581..49e2e947b36b 100644
--- a/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c
+++ b/drivers/crypto/allwinner/sun8i-ss/sun8i-ss-hash.c
@@ -420,15 +420,15 @@ int sun8i_ss_hash_run(struct crypto_engine *engine, void *breq)
}
addr_pad = dma_map_single(ss->dev, pad, j * 4, DMA_TO_DEVICE);
- rctx->t_src[i].addr = addr_pad;
- rctx->t_src[i].len = j;
- rctx->t_dst[i].addr = addr_res;
- rctx->t_dst[i].len = digestsize / 4;
if (dma_mapping_error(ss->dev, addr_pad)) {
dev_err(ss->dev, "DMA error on padding SG\n");
err = -EINVAL;
goto theend;
}
+ rctx->t_src[i].addr = addr_pad;
+ rctx->t_src[i].len = j;
+ rctx->t_dst[i].addr = addr_res;
+ rctx->t_dst[i].len = digestsize / 4;
err = sun8i_ss_run_hash_task(ss, rctx, crypto_tfm_alg_name(areq->base.tfm));
--
2.34.1
Le Mon, Apr 11, 2022 at 03:40:28PM +0800, Herbert Xu a ?crit :
> On Mon, Apr 11, 2022 at 09:37:22AM +0200, LABBE Corentin wrote:
> >
> > Coul you give me more details ?
> > I do not have any sparse error.
>
> Did you compile with C=1? Anyway, for a start hash_pad is broken as
> it tries to store an le32 value into a u32.
Yes I compile with both W=1 and C=1.
But rigth, hash_pad takes a u32 but the origin variable bf is __le32, so perhaps it is why sparse dont see a problem.
Anyway I will fix hash_pad declaration.
Thanks