* [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback
@ 2019-10-05 9:11 Ard Biesheuvel
2019-10-05 12:22 ` Florian Bezdeka
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2019-10-05 9:11 UTC (permalink / raw)
To: linux-crypto; +Cc: t2, jelledejong, ebiggers, florian, herbert, Ard Biesheuvel
Commit 79c65d179a40e145 ("crypto: cbc - Convert to skcipher") updated
the generic CBC template wrapper from a blkcipher to a skcipher algo,
to get away from the deprecated blkcipher interface. However, as a side
effect, drivers that instantiate CBC transforms using the blkcipher as
a fallback no longer work, since skciphers can wrap blkciphers but not
the other way around. This broke the geode-aes driver.
So let's fix it by moving to the sync skcipher interface when allocating
the fallback. At the same time, align with the generic API for ECB and
CBC by rejecting inputs that are not a multiple of the AES block size.
Fixes: 79c65d179a40e145 ("crypto: cbc - Convert to skcipher")
Cc: <stable@vger.kernel.org> # v4.20+ ONLY
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: pass dst and src scatterlist in the right order
reject inputs that are not a multiple of the block size
drivers/crypto/geode-aes.c | 57 +++++++++++---------
drivers/crypto/geode-aes.h | 2 +-
2 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index d81a1297cb9e..940485112d15 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -10,6 +10,7 @@
#include <linux/spinlock.h>
#include <crypto/algapi.h>
#include <crypto/aes.h>
+#include <crypto/skcipher.h>
#include <linux/io.h>
#include <linux/delay.h>
@@ -166,13 +167,15 @@ static int geode_setkey_blk(struct crypto_tfm *tfm, const u8 *key,
/*
* The requested key size is not supported by HW, do a fallback
*/
- op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK;
- op->fallback.blk->base.crt_flags |= (tfm->crt_flags & CRYPTO_TFM_REQ_MASK);
+ crypto_sync_skcipher_clear_flags(op->fallback.blk, CRYPTO_TFM_REQ_MASK);
+ crypto_sync_skcipher_set_flags(op->fallback.blk,
+ tfm->crt_flags & CRYPTO_TFM_REQ_MASK);
- ret = crypto_blkcipher_setkey(op->fallback.blk, key, len);
+ ret = crypto_sync_skcipher_setkey(op->fallback.blk, key, len);
if (ret) {
tfm->crt_flags &= ~CRYPTO_TFM_RES_MASK;
- tfm->crt_flags |= (op->fallback.blk->base.crt_flags & CRYPTO_TFM_RES_MASK);
+ tfm->crt_flags |= crypto_sync_skcipher_get_flags(op->fallback.blk) &
+ CRYPTO_TFM_RES_MASK;
}
return ret;
}
@@ -181,33 +184,28 @@ static int fallback_blk_dec(struct blkcipher_desc *desc,
struct scatterlist *dst, struct scatterlist *src,
unsigned int nbytes)
{
- unsigned int ret;
- struct crypto_blkcipher *tfm;
struct geode_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
+ SYNC_SKCIPHER_REQUEST_ON_STACK(req, op->fallback.blk);
- tfm = desc->tfm;
- desc->tfm = op->fallback.blk;
-
- ret = crypto_blkcipher_decrypt_iv(desc, dst, src, nbytes);
+ skcipher_request_set_sync_tfm(req, op->fallback.blk);
+ skcipher_request_set_callback(req, 0, NULL, NULL);
+ skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
- desc->tfm = tfm;
- return ret;
+ return crypto_skcipher_decrypt(req);
}
+
static int fallback_blk_enc(struct blkcipher_desc *desc,
struct scatterlist *dst, struct scatterlist *src,
unsigned int nbytes)
{
- unsigned int ret;
- struct crypto_blkcipher *tfm;
struct geode_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
+ SYNC_SKCIPHER_REQUEST_ON_STACK(req, op->fallback.blk);
- tfm = desc->tfm;
- desc->tfm = op->fallback.blk;
-
- ret = crypto_blkcipher_encrypt_iv(desc, dst, src, nbytes);
+ skcipher_request_set_sync_tfm(req, op->fallback.blk);
+ skcipher_request_set_callback(req, 0, NULL, NULL);
+ skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
- desc->tfm = tfm;
- return ret;
+ return crypto_skcipher_encrypt(req);
}
static void
@@ -307,6 +305,9 @@ geode_cbc_decrypt(struct blkcipher_desc *desc,
struct blkcipher_walk walk;
int err, ret;
+ if (nbytes % AES_BLOCK_SIZE)
+ return -EINVAL;
+
if (unlikely(op->keylen != AES_KEYSIZE_128))
return fallback_blk_dec(desc, dst, src, nbytes);
@@ -339,6 +340,9 @@ geode_cbc_encrypt(struct blkcipher_desc *desc,
struct blkcipher_walk walk;
int err, ret;
+ if (nbytes % AES_BLOCK_SIZE)
+ return -EINVAL;
+
if (unlikely(op->keylen != AES_KEYSIZE_128))
return fallback_blk_enc(desc, dst, src, nbytes);
@@ -366,9 +370,8 @@ static int fallback_init_blk(struct crypto_tfm *tfm)
const char *name = crypto_tfm_alg_name(tfm);
struct geode_aes_op *op = crypto_tfm_ctx(tfm);
- op->fallback.blk = crypto_alloc_blkcipher(name, 0,
- CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
-
+ op->fallback.blk = crypto_alloc_sync_skcipher(name, 0,
+ CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(op->fallback.blk)) {
printk(KERN_ERR "Error allocating fallback algo %s\n", name);
return PTR_ERR(op->fallback.blk);
@@ -381,7 +384,7 @@ static void fallback_exit_blk(struct crypto_tfm *tfm)
{
struct geode_aes_op *op = crypto_tfm_ctx(tfm);
- crypto_free_blkcipher(op->fallback.blk);
+ crypto_free_sync_skcipher(op->fallback.blk);
op->fallback.blk = NULL;
}
@@ -420,6 +423,9 @@ geode_ecb_decrypt(struct blkcipher_desc *desc,
struct blkcipher_walk walk;
int err, ret;
+ if (nbytes % AES_BLOCK_SIZE)
+ return -EINVAL;
+
if (unlikely(op->keylen != AES_KEYSIZE_128))
return fallback_blk_dec(desc, dst, src, nbytes);
@@ -450,6 +456,9 @@ geode_ecb_encrypt(struct blkcipher_desc *desc,
struct blkcipher_walk walk;
int err, ret;
+ if (nbytes % AES_BLOCK_SIZE)
+ return -EINVAL;
+
if (unlikely(op->keylen != AES_KEYSIZE_128))
return fallback_blk_enc(desc, dst, src, nbytes);
diff --git a/drivers/crypto/geode-aes.h b/drivers/crypto/geode-aes.h
index 5c6e131a8f9d..f8a86898ac22 100644
--- a/drivers/crypto/geode-aes.h
+++ b/drivers/crypto/geode-aes.h
@@ -60,7 +60,7 @@ struct geode_aes_op {
u8 *iv;
union {
- struct crypto_blkcipher *blk;
+ struct crypto_sync_skcipher *blk;
struct crypto_cipher *cip;
} fallback;
u32 keylen;
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback
2019-10-05 9:11 [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback Ard Biesheuvel
@ 2019-10-05 12:22 ` Florian Bezdeka
2019-10-08 11:36 ` Ard Biesheuvel
2019-10-05 16:15 ` Gert Robben
2019-10-10 12:58 ` Herbert Xu
2 siblings, 1 reply; 8+ messages in thread
From: Florian Bezdeka @ 2019-10-05 12:22 UTC (permalink / raw)
To: linux-crypto; +Cc: Florian Bezdeka, Ard Biesheuvel
Commit 79c65d179a40e145 ("crypto: cbc - Convert to skcipher") updated
the generic CBC template wrapper from a blkcipher to a skcipher algo,
to get away from the deprecated blkcipher interface. However, as a side
effect, drivers that instantiate CBC transforms using the blkcipher as
a fallback no longer work, since skciphers can wrap blkciphers but not
the other way around. This broke the geode-aes driver.
So let's fix it by moving to the sync skcipher interface when allocating
the fallback. At the same time, align with the generic API for ECB and
CBC by rejecting inputs that are not a multiple of the AES block size.
Fixes: 79c65d179a40e145 ("crypto: cbc - Convert to skcipher")
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Florian Bezdeka <florian@bezdeka.de>
---
Ard, I just followed your instructions and created this patch
for usage on an 4.19 kernel. The patch was successfully tested
on two different Geode systems.
Can you please review again and forward to the stable tree if the patch
looks OK?
drivers/crypto/geode-aes.c | 57 +++++++++++++++++++++++---------------
drivers/crypto/geode-aes.h | 2 +-
2 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index eb2a0a73cbed..cc33354d13c1 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -14,6 +14,7 @@
#include <linux/spinlock.h>
#include <crypto/algapi.h>
#include <crypto/aes.h>
+#include <crypto/skcipher.h>
#include <linux/io.h>
#include <linux/delay.h>
@@ -170,13 +171,15 @@ static int geode_setkey_blk(struct crypto_tfm *tfm, const u8 *key,
/*
* The requested key size is not supported by HW, do a fallback
*/
- op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK;
- op->fallback.blk->base.crt_flags |= (tfm->crt_flags & CRYPTO_TFM_REQ_MASK);
+ crypto_skcipher_clear_flags(op->fallback.blk, CRYPTO_TFM_REQ_MASK);
+ crypto_skcipher_set_flags(op->fallback.blk,
+ tfm->crt_flags & CRYPTO_TFM_REQ_MASK);
- ret = crypto_blkcipher_setkey(op->fallback.blk, key, len);
+ ret = crypto_skcipher_setkey(op->fallback.blk, key, len);
if (ret) {
tfm->crt_flags &= ~CRYPTO_TFM_RES_MASK;
- tfm->crt_flags |= (op->fallback.blk->base.crt_flags & CRYPTO_TFM_RES_MASK);
+ tfm->crt_flags |= crypto_skcipher_get_flags(op->fallback.blk) &
+ CRYPTO_TFM_RES_MASK;
}
return ret;
}
@@ -185,33 +188,28 @@ static int fallback_blk_dec(struct blkcipher_desc *desc,
struct scatterlist *dst, struct scatterlist *src,
unsigned int nbytes)
{
- unsigned int ret;
- struct crypto_blkcipher *tfm;
struct geode_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
+ SKCIPHER_REQUEST_ON_STACK(req, op->fallback.blk);
- tfm = desc->tfm;
- desc->tfm = op->fallback.blk;
-
- ret = crypto_blkcipher_decrypt_iv(desc, dst, src, nbytes);
+ skcipher_request_set_tfm(req, op->fallback.blk);
+ skcipher_request_set_callback(req, 0, NULL, NULL);
+ skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
- desc->tfm = tfm;
- return ret;
+ return crypto_skcipher_decrypt(req);
}
+
static int fallback_blk_enc(struct blkcipher_desc *desc,
struct scatterlist *dst, struct scatterlist *src,
unsigned int nbytes)
{
- unsigned int ret;
- struct crypto_blkcipher *tfm;
struct geode_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
+ SKCIPHER_REQUEST_ON_STACK(req, op->fallback.blk);
- tfm = desc->tfm;
- desc->tfm = op->fallback.blk;
-
- ret = crypto_blkcipher_encrypt_iv(desc, dst, src, nbytes);
+ skcipher_request_set_tfm(req, op->fallback.blk);
+ skcipher_request_set_callback(req, 0, NULL, NULL);
+ skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
- desc->tfm = tfm;
- return ret;
+ return crypto_skcipher_encrypt(req);
}
static void
@@ -311,6 +309,9 @@ geode_cbc_decrypt(struct blkcipher_desc *desc,
struct blkcipher_walk walk;
int err, ret;
+ if (nbytes % AES_BLOCK_SIZE)
+ return -EINVAL;
+
if (unlikely(op->keylen != AES_KEYSIZE_128))
return fallback_blk_dec(desc, dst, src, nbytes);
@@ -343,6 +344,9 @@ geode_cbc_encrypt(struct blkcipher_desc *desc,
struct blkcipher_walk walk;
int err, ret;
+ if (nbytes % AES_BLOCK_SIZE)
+ return -EINVAL;
+
if (unlikely(op->keylen != AES_KEYSIZE_128))
return fallback_blk_enc(desc, dst, src, nbytes);
@@ -370,8 +374,9 @@ static int fallback_init_blk(struct crypto_tfm *tfm)
const char *name = crypto_tfm_alg_name(tfm);
struct geode_aes_op *op = crypto_tfm_ctx(tfm);
- op->fallback.blk = crypto_alloc_blkcipher(name, 0,
- CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
+ op->fallback.blk = crypto_alloc_skcipher(name, 0,
+ CRYPTO_ALG_ASYNC |
+ CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(op->fallback.blk)) {
printk(KERN_ERR "Error allocating fallback algo %s\n", name);
@@ -385,7 +390,7 @@ static void fallback_exit_blk(struct crypto_tfm *tfm)
{
struct geode_aes_op *op = crypto_tfm_ctx(tfm);
- crypto_free_blkcipher(op->fallback.blk);
+ crypto_free_skcipher(op->fallback.blk);
op->fallback.blk = NULL;
}
@@ -424,6 +429,9 @@ geode_ecb_decrypt(struct blkcipher_desc *desc,
struct blkcipher_walk walk;
int err, ret;
+ if (nbytes % AES_BLOCK_SIZE)
+ return -EINVAL;
+
if (unlikely(op->keylen != AES_KEYSIZE_128))
return fallback_blk_dec(desc, dst, src, nbytes);
@@ -454,6 +462,9 @@ geode_ecb_encrypt(struct blkcipher_desc *desc,
struct blkcipher_walk walk;
int err, ret;
+ if (nbytes % AES_BLOCK_SIZE)
+ return -EINVAL;
+
if (unlikely(op->keylen != AES_KEYSIZE_128))
return fallback_blk_enc(desc, dst, src, nbytes);
diff --git a/drivers/crypto/geode-aes.h b/drivers/crypto/geode-aes.h
index f442ca972e3c..c5763a041bb8 100644
--- a/drivers/crypto/geode-aes.h
+++ b/drivers/crypto/geode-aes.h
@@ -64,7 +64,7 @@ struct geode_aes_op {
u8 *iv;
union {
- struct crypto_blkcipher *blk;
+ struct crypto_skcipher *blk;
struct crypto_cipher *cip;
} fallback;
u32 keylen;
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback
2019-10-05 12:22 ` Florian Bezdeka
@ 2019-10-08 11:36 ` Ard Biesheuvel
2020-01-19 10:02 ` Florian Bezdeka
0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2019-10-08 11:36 UTC (permalink / raw)
To: Florian Bezdeka; +Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE
On Sat, 5 Oct 2019 at 14:22, Florian Bezdeka <florian@bezdeka.de> wrote:
>
> Commit 79c65d179a40e145 ("crypto: cbc - Convert to skcipher") updated
> the generic CBC template wrapper from a blkcipher to a skcipher algo,
> to get away from the deprecated blkcipher interface. However, as a side
> effect, drivers that instantiate CBC transforms using the blkcipher as
> a fallback no longer work, since skciphers can wrap blkciphers but not
> the other way around. This broke the geode-aes driver.
>
> So let's fix it by moving to the sync skcipher interface when allocating
> the fallback. At the same time, align with the generic API for ECB and
> CBC by rejecting inputs that are not a multiple of the AES block size.
>
> Fixes: 79c65d179a40e145 ("crypto: cbc - Convert to skcipher")
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Florian Bezdeka <florian@bezdeka.de>
> ---
>
> Ard, I just followed your instructions and created this patch
> for usage on an 4.19 kernel. The patch was successfully tested
> on two different Geode systems.
>
> Can you please review again and forward to the stable tree if the patch
> looks OK?
>
The patch looks fine to me, but we cannot send it to -stable before
the mainline version is merged.
> drivers/crypto/geode-aes.c | 57 +++++++++++++++++++++++---------------
> drivers/crypto/geode-aes.h | 2 +-
> 2 files changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
> index eb2a0a73cbed..cc33354d13c1 100644
> --- a/drivers/crypto/geode-aes.c
> +++ b/drivers/crypto/geode-aes.c
> @@ -14,6 +14,7 @@
> #include <linux/spinlock.h>
> #include <crypto/algapi.h>
> #include <crypto/aes.h>
> +#include <crypto/skcipher.h>
>
> #include <linux/io.h>
> #include <linux/delay.h>
> @@ -170,13 +171,15 @@ static int geode_setkey_blk(struct crypto_tfm *tfm, const u8 *key,
> /*
> * The requested key size is not supported by HW, do a fallback
> */
> - op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK;
> - op->fallback.blk->base.crt_flags |= (tfm->crt_flags & CRYPTO_TFM_REQ_MASK);
> + crypto_skcipher_clear_flags(op->fallback.blk, CRYPTO_TFM_REQ_MASK);
> + crypto_skcipher_set_flags(op->fallback.blk,
> + tfm->crt_flags & CRYPTO_TFM_REQ_MASK);
>
> - ret = crypto_blkcipher_setkey(op->fallback.blk, key, len);
> + ret = crypto_skcipher_setkey(op->fallback.blk, key, len);
> if (ret) {
> tfm->crt_flags &= ~CRYPTO_TFM_RES_MASK;
> - tfm->crt_flags |= (op->fallback.blk->base.crt_flags & CRYPTO_TFM_RES_MASK);
> + tfm->crt_flags |= crypto_skcipher_get_flags(op->fallback.blk) &
> + CRYPTO_TFM_RES_MASK;
> }
> return ret;
> }
> @@ -185,33 +188,28 @@ static int fallback_blk_dec(struct blkcipher_desc *desc,
> struct scatterlist *dst, struct scatterlist *src,
> unsigned int nbytes)
> {
> - unsigned int ret;
> - struct crypto_blkcipher *tfm;
> struct geode_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
> + SKCIPHER_REQUEST_ON_STACK(req, op->fallback.blk);
>
> - tfm = desc->tfm;
> - desc->tfm = op->fallback.blk;
> -
> - ret = crypto_blkcipher_decrypt_iv(desc, dst, src, nbytes);
> + skcipher_request_set_tfm(req, op->fallback.blk);
> + skcipher_request_set_callback(req, 0, NULL, NULL);
> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
>
> - desc->tfm = tfm;
> - return ret;
> + return crypto_skcipher_decrypt(req);
> }
> +
> static int fallback_blk_enc(struct blkcipher_desc *desc,
> struct scatterlist *dst, struct scatterlist *src,
> unsigned int nbytes)
> {
> - unsigned int ret;
> - struct crypto_blkcipher *tfm;
> struct geode_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
> + SKCIPHER_REQUEST_ON_STACK(req, op->fallback.blk);
>
> - tfm = desc->tfm;
> - desc->tfm = op->fallback.blk;
> -
> - ret = crypto_blkcipher_encrypt_iv(desc, dst, src, nbytes);
> + skcipher_request_set_tfm(req, op->fallback.blk);
> + skcipher_request_set_callback(req, 0, NULL, NULL);
> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
>
> - desc->tfm = tfm;
> - return ret;
> + return crypto_skcipher_encrypt(req);
> }
>
> static void
> @@ -311,6 +309,9 @@ geode_cbc_decrypt(struct blkcipher_desc *desc,
> struct blkcipher_walk walk;
> int err, ret;
>
> + if (nbytes % AES_BLOCK_SIZE)
> + return -EINVAL;
> +
> if (unlikely(op->keylen != AES_KEYSIZE_128))
> return fallback_blk_dec(desc, dst, src, nbytes);
>
> @@ -343,6 +344,9 @@ geode_cbc_encrypt(struct blkcipher_desc *desc,
> struct blkcipher_walk walk;
> int err, ret;
>
> + if (nbytes % AES_BLOCK_SIZE)
> + return -EINVAL;
> +
> if (unlikely(op->keylen != AES_KEYSIZE_128))
> return fallback_blk_enc(desc, dst, src, nbytes);
>
> @@ -370,8 +374,9 @@ static int fallback_init_blk(struct crypto_tfm *tfm)
> const char *name = crypto_tfm_alg_name(tfm);
> struct geode_aes_op *op = crypto_tfm_ctx(tfm);
>
> - op->fallback.blk = crypto_alloc_blkcipher(name, 0,
> - CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
> + op->fallback.blk = crypto_alloc_skcipher(name, 0,
> + CRYPTO_ALG_ASYNC |
> + CRYPTO_ALG_NEED_FALLBACK);
>
> if (IS_ERR(op->fallback.blk)) {
> printk(KERN_ERR "Error allocating fallback algo %s\n", name);
> @@ -385,7 +390,7 @@ static void fallback_exit_blk(struct crypto_tfm *tfm)
> {
> struct geode_aes_op *op = crypto_tfm_ctx(tfm);
>
> - crypto_free_blkcipher(op->fallback.blk);
> + crypto_free_skcipher(op->fallback.blk);
> op->fallback.blk = NULL;
> }
>
> @@ -424,6 +429,9 @@ geode_ecb_decrypt(struct blkcipher_desc *desc,
> struct blkcipher_walk walk;
> int err, ret;
>
> + if (nbytes % AES_BLOCK_SIZE)
> + return -EINVAL;
> +
> if (unlikely(op->keylen != AES_KEYSIZE_128))
> return fallback_blk_dec(desc, dst, src, nbytes);
>
> @@ -454,6 +462,9 @@ geode_ecb_encrypt(struct blkcipher_desc *desc,
> struct blkcipher_walk walk;
> int err, ret;
>
> + if (nbytes % AES_BLOCK_SIZE)
> + return -EINVAL;
> +
> if (unlikely(op->keylen != AES_KEYSIZE_128))
> return fallback_blk_enc(desc, dst, src, nbytes);
>
> diff --git a/drivers/crypto/geode-aes.h b/drivers/crypto/geode-aes.h
> index f442ca972e3c..c5763a041bb8 100644
> --- a/drivers/crypto/geode-aes.h
> +++ b/drivers/crypto/geode-aes.h
> @@ -64,7 +64,7 @@ struct geode_aes_op {
> u8 *iv;
>
> union {
> - struct crypto_blkcipher *blk;
> + struct crypto_skcipher *blk;
> struct crypto_cipher *cip;
> } fallback;
> u32 keylen;
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback
2019-10-08 11:36 ` Ard Biesheuvel
@ 2020-01-19 10:02 ` Florian Bezdeka
2020-01-19 13:51 ` Ard Biesheuvel
0 siblings, 1 reply; 8+ messages in thread
From: Florian Bezdeka @ 2020-01-19 10:02 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE
Hi Ard,
Greg picked the non-backported version of the v2 patch and merged it into the
4.9, 4.14 and 4.19 stable trees. The commit of the stable-queue repository [1]
is [2].
Some days later he noticed that the non-backported version is not working as
expected (failing the build) and he removed the patch from all affected stable
trees again. The commit id is [3].
What are the right steps to get the backported version of this patch to the
stable-queue and afterwards to the stable trees?
Thank you,
Florian
[1] git://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git
[2] ab6793e0731db6b937c47faf2ad95c5d9ef9ff86
[3] 23e4ec1db5b3ba3fb5cb60aac0b9b84e244e0a10
Am 08.10.19 um 13:36 schrieb Ard Biesheuvel:
> On Sat, 5 Oct 2019 at 14:22, Florian Bezdeka <florian@bezdeka.de> wrote:
>>
>> Commit 79c65d179a40e145 ("crypto: cbc - Convert to skcipher") updated
>> the generic CBC template wrapper from a blkcipher to a skcipher algo,
>> to get away from the deprecated blkcipher interface. However, as a side
>> effect, drivers that instantiate CBC transforms using the blkcipher as
>> a fallback no longer work, since skciphers can wrap blkciphers but not
>> the other way around. This broke the geode-aes driver.
>>
>> So let's fix it by moving to the sync skcipher interface when allocating
>> the fallback. At the same time, align with the generic API for ECB and
>> CBC by rejecting inputs that are not a multiple of the AES block size.
>>
>> Fixes: 79c65d179a40e145 ("crypto: cbc - Convert to skcipher")
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Signed-off-by: Florian Bezdeka <florian@bezdeka.de>
>> ---
>>
>> Ard, I just followed your instructions and created this patch
>> for usage on an 4.19 kernel. The patch was successfully tested
>> on two different Geode systems.
>>
>> Can you please review again and forward to the stable tree if the patch
>> looks OK?
>>
>
> The patch looks fine to me, but we cannot send it to -stable before
> the mainline version is merged.
>
>> drivers/crypto/geode-aes.c | 57 +++++++++++++++++++++++---------------
>> drivers/crypto/geode-aes.h | 2 +-
>> 2 files changed, 35 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
>> index eb2a0a73cbed..cc33354d13c1 100644
>> --- a/drivers/crypto/geode-aes.c
>> +++ b/drivers/crypto/geode-aes.c
>> @@ -14,6 +14,7 @@
>> #include <linux/spinlock.h>
>> #include <crypto/algapi.h>
>> #include <crypto/aes.h>
>> +#include <crypto/skcipher.h>
>>
>> #include <linux/io.h>
>> #include <linux/delay.h>
>> @@ -170,13 +171,15 @@ static int geode_setkey_blk(struct crypto_tfm *tfm, const u8 *key,
>> /*
>> * The requested key size is not supported by HW, do a fallback
>> */
>> - op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK;
>> - op->fallback.blk->base.crt_flags |= (tfm->crt_flags & CRYPTO_TFM_REQ_MASK);
>> + crypto_skcipher_clear_flags(op->fallback.blk, CRYPTO_TFM_REQ_MASK);
>> + crypto_skcipher_set_flags(op->fallback.blk,
>> + tfm->crt_flags & CRYPTO_TFM_REQ_MASK);
>>
>> - ret = crypto_blkcipher_setkey(op->fallback.blk, key, len);
>> + ret = crypto_skcipher_setkey(op->fallback.blk, key, len);
>> if (ret) {
>> tfm->crt_flags &= ~CRYPTO_TFM_RES_MASK;
>> - tfm->crt_flags |= (op->fallback.blk->base.crt_flags & CRYPTO_TFM_RES_MASK);
>> + tfm->crt_flags |= crypto_skcipher_get_flags(op->fallback.blk) &
>> + CRYPTO_TFM_RES_MASK;
>> }
>> return ret;
>> }
>> @@ -185,33 +188,28 @@ static int fallback_blk_dec(struct blkcipher_desc *desc,
>> struct scatterlist *dst, struct scatterlist *src,
>> unsigned int nbytes)
>> {
>> - unsigned int ret;
>> - struct crypto_blkcipher *tfm;
>> struct geode_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
>> + SKCIPHER_REQUEST_ON_STACK(req, op->fallback.blk);
>>
>> - tfm = desc->tfm;
>> - desc->tfm = op->fallback.blk;
>> -
>> - ret = crypto_blkcipher_decrypt_iv(desc, dst, src, nbytes);
>> + skcipher_request_set_tfm(req, op->fallback.blk);
>> + skcipher_request_set_callback(req, 0, NULL, NULL);
>> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
>>
>> - desc->tfm = tfm;
>> - return ret;
>> + return crypto_skcipher_decrypt(req);
>> }
>> +
>> static int fallback_blk_enc(struct blkcipher_desc *desc,
>> struct scatterlist *dst, struct scatterlist *src,
>> unsigned int nbytes)
>> {
>> - unsigned int ret;
>> - struct crypto_blkcipher *tfm;
>> struct geode_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
>> + SKCIPHER_REQUEST_ON_STACK(req, op->fallback.blk);
>>
>> - tfm = desc->tfm;
>> - desc->tfm = op->fallback.blk;
>> -
>> - ret = crypto_blkcipher_encrypt_iv(desc, dst, src, nbytes);
>> + skcipher_request_set_tfm(req, op->fallback.blk);
>> + skcipher_request_set_callback(req, 0, NULL, NULL);
>> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
>>
>> - desc->tfm = tfm;
>> - return ret;
>> + return crypto_skcipher_encrypt(req);
>> }
>>
>> static void
>> @@ -311,6 +309,9 @@ geode_cbc_decrypt(struct blkcipher_desc *desc,
>> struct blkcipher_walk walk;
>> int err, ret;
>>
>> + if (nbytes % AES_BLOCK_SIZE)
>> + return -EINVAL;
>> +
>> if (unlikely(op->keylen != AES_KEYSIZE_128))
>> return fallback_blk_dec(desc, dst, src, nbytes);
>>
>> @@ -343,6 +344,9 @@ geode_cbc_encrypt(struct blkcipher_desc *desc,
>> struct blkcipher_walk walk;
>> int err, ret;
>>
>> + if (nbytes % AES_BLOCK_SIZE)
>> + return -EINVAL;
>> +
>> if (unlikely(op->keylen != AES_KEYSIZE_128))
>> return fallback_blk_enc(desc, dst, src, nbytes);
>>
>> @@ -370,8 +374,9 @@ static int fallback_init_blk(struct crypto_tfm *tfm)
>> const char *name = crypto_tfm_alg_name(tfm);
>> struct geode_aes_op *op = crypto_tfm_ctx(tfm);
>>
>> - op->fallback.blk = crypto_alloc_blkcipher(name, 0,
>> - CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
>> + op->fallback.blk = crypto_alloc_skcipher(name, 0,
>> + CRYPTO_ALG_ASYNC |
>> + CRYPTO_ALG_NEED_FALLBACK);
>>
>> if (IS_ERR(op->fallback.blk)) {
>> printk(KERN_ERR "Error allocating fallback algo %s\n", name);
>> @@ -385,7 +390,7 @@ static void fallback_exit_blk(struct crypto_tfm *tfm)
>> {
>> struct geode_aes_op *op = crypto_tfm_ctx(tfm);
>>
>> - crypto_free_blkcipher(op->fallback.blk);
>> + crypto_free_skcipher(op->fallback.blk);
>> op->fallback.blk = NULL;
>> }
>>
>> @@ -424,6 +429,9 @@ geode_ecb_decrypt(struct blkcipher_desc *desc,
>> struct blkcipher_walk walk;
>> int err, ret;
>>
>> + if (nbytes % AES_BLOCK_SIZE)
>> + return -EINVAL;
>> +
>> if (unlikely(op->keylen != AES_KEYSIZE_128))
>> return fallback_blk_dec(desc, dst, src, nbytes);
>>
>> @@ -454,6 +462,9 @@ geode_ecb_encrypt(struct blkcipher_desc *desc,
>> struct blkcipher_walk walk;
>> int err, ret;
>>
>> + if (nbytes % AES_BLOCK_SIZE)
>> + return -EINVAL;
>> +
>> if (unlikely(op->keylen != AES_KEYSIZE_128))
>> return fallback_blk_enc(desc, dst, src, nbytes);
>>
>> diff --git a/drivers/crypto/geode-aes.h b/drivers/crypto/geode-aes.h
>> index f442ca972e3c..c5763a041bb8 100644
>> --- a/drivers/crypto/geode-aes.h
>> +++ b/drivers/crypto/geode-aes.h
>> @@ -64,7 +64,7 @@ struct geode_aes_op {
>> u8 *iv;
>>
>> union {
>> - struct crypto_blkcipher *blk;
>> + struct crypto_skcipher *blk;
>> struct crypto_cipher *cip;
>> } fallback;
>> u32 keylen;
>> --
>> 2.21.0
>>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback
2020-01-19 10:02 ` Florian Bezdeka
@ 2020-01-19 13:51 ` Ard Biesheuvel
0 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2020-01-19 13:51 UTC (permalink / raw)
To: Florian Bezdeka; +Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE
On Sun, 19 Jan 2020 at 11:02, Florian Bezdeka <florian@bezdeka.de> wrote:
>
> Hi Ard,
>
> Greg picked the non-backported version of the v2 patch and merged it into the
> 4.9, 4.14 and 4.19 stable trees. The commit of the stable-queue repository [1]
> is [2].
>
> Some days later he noticed that the non-backported version is not working as
> expected (failing the build) and he removed the patch from all affected stable
> trees again. The commit id is [3].
>
> What are the right steps to get the backported version of this patch to the
> stable-queue and afterwards to the stable trees?
>
Just send a working version of the patch to stable@vger.kernel.org,
and explain what needed to be changed to make it build correctly.
Look at stable-kernel-rules in the kernel Documentation/ directory for
instructions
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git
> [2] ab6793e0731db6b937c47faf2ad95c5d9ef9ff86
> [3] 23e4ec1db5b3ba3fb5cb60aac0b9b84e244e0a10
>
> Am 08.10.19 um 13:36 schrieb Ard Biesheuvel:
> > On Sat, 5 Oct 2019 at 14:22, Florian Bezdeka <florian@bezdeka.de> wrote:
> >>
> >> Commit 79c65d179a40e145 ("crypto: cbc - Convert to skcipher") updated
> >> the generic CBC template wrapper from a blkcipher to a skcipher algo,
> >> to get away from the deprecated blkcipher interface. However, as a side
> >> effect, drivers that instantiate CBC transforms using the blkcipher as
> >> a fallback no longer work, since skciphers can wrap blkciphers but not
> >> the other way around. This broke the geode-aes driver.
> >>
> >> So let's fix it by moving to the sync skcipher interface when allocating
> >> the fallback. At the same time, align with the generic API for ECB and
> >> CBC by rejecting inputs that are not a multiple of the AES block size.
> >>
> >> Fixes: 79c65d179a40e145 ("crypto: cbc - Convert to skcipher")
> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Signed-off-by: Florian Bezdeka <florian@bezdeka.de>
> >> ---
> >>
> >> Ard, I just followed your instructions and created this patch
> >> for usage on an 4.19 kernel. The patch was successfully tested
> >> on two different Geode systems.
> >>
> >> Can you please review again and forward to the stable tree if the patch
> >> looks OK?
> >>
> >
> > The patch looks fine to me, but we cannot send it to -stable before
> > the mainline version is merged.
> >
> >> drivers/crypto/geode-aes.c | 57 +++++++++++++++++++++++---------------
> >> drivers/crypto/geode-aes.h | 2 +-
> >> 2 files changed, 35 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
> >> index eb2a0a73cbed..cc33354d13c1 100644
> >> --- a/drivers/crypto/geode-aes.c
> >> +++ b/drivers/crypto/geode-aes.c
> >> @@ -14,6 +14,7 @@
> >> #include <linux/spinlock.h>
> >> #include <crypto/algapi.h>
> >> #include <crypto/aes.h>
> >> +#include <crypto/skcipher.h>
> >>
> >> #include <linux/io.h>
> >> #include <linux/delay.h>
> >> @@ -170,13 +171,15 @@ static int geode_setkey_blk(struct crypto_tfm *tfm, const u8 *key,
> >> /*
> >> * The requested key size is not supported by HW, do a fallback
> >> */
> >> - op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK;
> >> - op->fallback.blk->base.crt_flags |= (tfm->crt_flags & CRYPTO_TFM_REQ_MASK);
> >> + crypto_skcipher_clear_flags(op->fallback.blk, CRYPTO_TFM_REQ_MASK);
> >> + crypto_skcipher_set_flags(op->fallback.blk,
> >> + tfm->crt_flags & CRYPTO_TFM_REQ_MASK);
> >>
> >> - ret = crypto_blkcipher_setkey(op->fallback.blk, key, len);
> >> + ret = crypto_skcipher_setkey(op->fallback.blk, key, len);
> >> if (ret) {
> >> tfm->crt_flags &= ~CRYPTO_TFM_RES_MASK;
> >> - tfm->crt_flags |= (op->fallback.blk->base.crt_flags & CRYPTO_TFM_RES_MASK);
> >> + tfm->crt_flags |= crypto_skcipher_get_flags(op->fallback.blk) &
> >> + CRYPTO_TFM_RES_MASK;
> >> }
> >> return ret;
> >> }
> >> @@ -185,33 +188,28 @@ static int fallback_blk_dec(struct blkcipher_desc *desc,
> >> struct scatterlist *dst, struct scatterlist *src,
> >> unsigned int nbytes)
> >> {
> >> - unsigned int ret;
> >> - struct crypto_blkcipher *tfm;
> >> struct geode_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
> >> + SKCIPHER_REQUEST_ON_STACK(req, op->fallback.blk);
> >>
> >> - tfm = desc->tfm;
> >> - desc->tfm = op->fallback.blk;
> >> -
> >> - ret = crypto_blkcipher_decrypt_iv(desc, dst, src, nbytes);
> >> + skcipher_request_set_tfm(req, op->fallback.blk);
> >> + skcipher_request_set_callback(req, 0, NULL, NULL);
> >> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
> >>
> >> - desc->tfm = tfm;
> >> - return ret;
> >> + return crypto_skcipher_decrypt(req);
> >> }
> >> +
> >> static int fallback_blk_enc(struct blkcipher_desc *desc,
> >> struct scatterlist *dst, struct scatterlist *src,
> >> unsigned int nbytes)
> >> {
> >> - unsigned int ret;
> >> - struct crypto_blkcipher *tfm;
> >> struct geode_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
> >> + SKCIPHER_REQUEST_ON_STACK(req, op->fallback.blk);
> >>
> >> - tfm = desc->tfm;
> >> - desc->tfm = op->fallback.blk;
> >> -
> >> - ret = crypto_blkcipher_encrypt_iv(desc, dst, src, nbytes);
> >> + skcipher_request_set_tfm(req, op->fallback.blk);
> >> + skcipher_request_set_callback(req, 0, NULL, NULL);
> >> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
> >>
> >> - desc->tfm = tfm;
> >> - return ret;
> >> + return crypto_skcipher_encrypt(req);
> >> }
> >>
> >> static void
> >> @@ -311,6 +309,9 @@ geode_cbc_decrypt(struct blkcipher_desc *desc,
> >> struct blkcipher_walk walk;
> >> int err, ret;
> >>
> >> + if (nbytes % AES_BLOCK_SIZE)
> >> + return -EINVAL;
> >> +
> >> if (unlikely(op->keylen != AES_KEYSIZE_128))
> >> return fallback_blk_dec(desc, dst, src, nbytes);
> >>
> >> @@ -343,6 +344,9 @@ geode_cbc_encrypt(struct blkcipher_desc *desc,
> >> struct blkcipher_walk walk;
> >> int err, ret;
> >>
> >> + if (nbytes % AES_BLOCK_SIZE)
> >> + return -EINVAL;
> >> +
> >> if (unlikely(op->keylen != AES_KEYSIZE_128))
> >> return fallback_blk_enc(desc, dst, src, nbytes);
> >>
> >> @@ -370,8 +374,9 @@ static int fallback_init_blk(struct crypto_tfm *tfm)
> >> const char *name = crypto_tfm_alg_name(tfm);
> >> struct geode_aes_op *op = crypto_tfm_ctx(tfm);
> >>
> >> - op->fallback.blk = crypto_alloc_blkcipher(name, 0,
> >> - CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
> >> + op->fallback.blk = crypto_alloc_skcipher(name, 0,
> >> + CRYPTO_ALG_ASYNC |
> >> + CRYPTO_ALG_NEED_FALLBACK);
> >>
> >> if (IS_ERR(op->fallback.blk)) {
> >> printk(KERN_ERR "Error allocating fallback algo %s\n", name);
> >> @@ -385,7 +390,7 @@ static void fallback_exit_blk(struct crypto_tfm *tfm)
> >> {
> >> struct geode_aes_op *op = crypto_tfm_ctx(tfm);
> >>
> >> - crypto_free_blkcipher(op->fallback.blk);
> >> + crypto_free_skcipher(op->fallback.blk);
> >> op->fallback.blk = NULL;
> >> }
> >>
> >> @@ -424,6 +429,9 @@ geode_ecb_decrypt(struct blkcipher_desc *desc,
> >> struct blkcipher_walk walk;
> >> int err, ret;
> >>
> >> + if (nbytes % AES_BLOCK_SIZE)
> >> + return -EINVAL;
> >> +
> >> if (unlikely(op->keylen != AES_KEYSIZE_128))
> >> return fallback_blk_dec(desc, dst, src, nbytes);
> >>
> >> @@ -454,6 +462,9 @@ geode_ecb_encrypt(struct blkcipher_desc *desc,
> >> struct blkcipher_walk walk;
> >> int err, ret;
> >>
> >> + if (nbytes % AES_BLOCK_SIZE)
> >> + return -EINVAL;
> >> +
> >> if (unlikely(op->keylen != AES_KEYSIZE_128))
> >> return fallback_blk_enc(desc, dst, src, nbytes);
> >>
> >> diff --git a/drivers/crypto/geode-aes.h b/drivers/crypto/geode-aes.h
> >> index f442ca972e3c..c5763a041bb8 100644
> >> --- a/drivers/crypto/geode-aes.h
> >> +++ b/drivers/crypto/geode-aes.h
> >> @@ -64,7 +64,7 @@ struct geode_aes_op {
> >> u8 *iv;
> >>
> >> union {
> >> - struct crypto_blkcipher *blk;
> >> + struct crypto_skcipher *blk;
> >> struct crypto_cipher *cip;
> >> } fallback;
> >> u32 keylen;
> >> --
> >> 2.21.0
> >>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback
2019-10-05 9:11 [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback Ard Biesheuvel
2019-10-05 12:22 ` Florian Bezdeka
@ 2019-10-05 16:15 ` Gert Robben
2019-10-08 11:38 ` Ard Biesheuvel
2019-10-10 12:58 ` Herbert Xu
2 siblings, 1 reply; 8+ messages in thread
From: Gert Robben @ 2019-10-05 16:15 UTC (permalink / raw)
To: Ard Biesheuvel, linux-crypto; +Cc: jelledejong, ebiggers, florian, herbert
Op 05-10-2019 om 11:11 schreef Ard Biesheuvel:
> Commit 79c65d179a40e145 ("crypto: cbc - Convert to skcipher") updated
> the generic CBC template wrapper from a blkcipher to a skcipher algo,
> to get away from the deprecated blkcipher interface. However, as a side
> effect, drivers that instantiate CBC transforms using the blkcipher as
> a fallback no longer work, since skciphers can wrap blkciphers but not
> the other way around. This broke the geode-aes driver.
>
> So let's fix it by moving to the sync skcipher interface when allocating
> the fallback. At the same time, align with the generic API for ECB and
> CBC by rejecting inputs that are not a multiple of the AES block size.
>
> Fixes: 79c65d179a40e145 ("crypto: cbc - Convert to skcipher")
> Cc: <stable@vger.kernel.org> # v4.20+ ONLY
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> v2: pass dst and src scatterlist in the right order
> reject inputs that are not a multiple of the block size
Yes, with this patch, the CRYPTO_MANAGER_EXTRA_TESTS output nothing
(apart from "extra crypto tests enabled").
All items in /proc/crypto have "selftest: passed" mentioned.
"openssl speed -evp aes-128-cbc -elapsed -engine afalg" reaches the
proper speed.
And nginx (correctly) transfers files about 40% faster than without
geode-aes.
I didn't think about testing ecb before, because I don't use it.
Now that I did, I tried the same openssl benchmark for ecb.
But that only reaches software AES speed, and "time" also shows the work
is being done in "user" instead of "sys" (see below).
Yet I see no errors.
(Maybe this is normal/expected, so I didn't look much further into it).
Thank you,
Gert
# time openssl speed -evp aes-128-cbc -elapsed -engine afalg
- - - 8< - - -
type 16 bytes 64 bytes 256 bytes 1024 bytes 8192
bytes 16384 bytes
aes-128-cbc 135.82k 539.29k 2087.90k 7491.16k
29221.69k 34943.67k
real 0m18.081s
user 0m0.516s
sys 0m17.541s
# time openssl speed -evp aes-128-ecb -elapsed -engine afalg
- - - 8< - - -
type 16 bytes 64 bytes 256 bytes 1024 bytes 8192
bytes 16384 bytes
aes-128-ecb 4480.65k 5137.66k 5336.94k 5410.19k
5409.91k 5409.91k
real 0m18.084s
user 0m18.046s
sys 0m0.012s
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback
2019-10-05 16:15 ` Gert Robben
@ 2019-10-08 11:38 ` Ard Biesheuvel
0 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2019-10-08 11:38 UTC (permalink / raw)
To: Gert Robben
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Jelle de Jong,
Eric Biggers, Florian Bezdeka, Herbert Xu
On Sat, 5 Oct 2019 at 18:15, Gert Robben <t2@gert.gr> wrote:
>
> Op 05-10-2019 om 11:11 schreef Ard Biesheuvel:
> > Commit 79c65d179a40e145 ("crypto: cbc - Convert to skcipher") updated
> > the generic CBC template wrapper from a blkcipher to a skcipher algo,
> > to get away from the deprecated blkcipher interface. However, as a side
> > effect, drivers that instantiate CBC transforms using the blkcipher as
> > a fallback no longer work, since skciphers can wrap blkciphers but not
> > the other way around. This broke the geode-aes driver.
> >
> > So let's fix it by moving to the sync skcipher interface when allocating
> > the fallback. At the same time, align with the generic API for ECB and
> > CBC by rejecting inputs that are not a multiple of the AES block size.
> >
> > Fixes: 79c65d179a40e145 ("crypto: cbc - Convert to skcipher")
> > Cc: <stable@vger.kernel.org> # v4.20+ ONLY
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > v2: pass dst and src scatterlist in the right order
> > reject inputs that are not a multiple of the block size
>
> Yes, with this patch, the CRYPTO_MANAGER_EXTRA_TESTS output nothing
> (apart from "extra crypto tests enabled").
> All items in /proc/crypto have "selftest: passed" mentioned.
> "openssl speed -evp aes-128-cbc -elapsed -engine afalg" reaches the
> proper speed.
> And nginx (correctly) transfers files about 40% faster than without
> geode-aes.
>
> I didn't think about testing ecb before, because I don't use it.
> Now that I did, I tried the same openssl benchmark for ecb.
> But that only reaches software AES speed, and "time" also shows the work
> is being done in "user" instead of "sys" (see below).
> Yet I see no errors.
> (Maybe this is normal/expected, so I didn't look much further into it).
>
> Thank you,
> Gert
>
> # time openssl speed -evp aes-128-cbc -elapsed -engine afalg
> - - - 8< - - -
> type 16 bytes 64 bytes 256 bytes 1024 bytes 8192
> bytes 16384 bytes
> aes-128-cbc 135.82k 539.29k 2087.90k 7491.16k
> 29221.69k 34943.67k
>
> real 0m18.081s
> user 0m0.516s
> sys 0m17.541s
>
> # time openssl speed -evp aes-128-ecb -elapsed -engine afalg
> - - - 8< - - -
> type 16 bytes 64 bytes 256 bytes 1024 bytes 8192
> bytes 16384 bytes
> aes-128-ecb 4480.65k 5137.66k 5336.94k 5410.19k
> 5409.91k 5409.91k
>
> real 0m18.084s
> user 0m18.046s
> sys 0m0.012s
>
It seems likely that the ECB code in OpenSSL is not invoking the afalg
code at all. Since ECB is just the bare block cipher applied to each
block in the input, I wonder if it even uses a skcipher like interface
internally.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback
2019-10-05 9:11 [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback Ard Biesheuvel
2019-10-05 12:22 ` Florian Bezdeka
2019-10-05 16:15 ` Gert Robben
@ 2019-10-10 12:58 ` Herbert Xu
2 siblings, 0 replies; 8+ messages in thread
From: Herbert Xu @ 2019-10-10 12:58 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-crypto, t2, jelledejong, ebiggers, florian
On Sat, Oct 05, 2019 at 11:11:10AM +0200, Ard Biesheuvel wrote:
> Commit 79c65d179a40e145 ("crypto: cbc - Convert to skcipher") updated
> the generic CBC template wrapper from a blkcipher to a skcipher algo,
> to get away from the deprecated blkcipher interface. However, as a side
> effect, drivers that instantiate CBC transforms using the blkcipher as
> a fallback no longer work, since skciphers can wrap blkciphers but not
> the other way around. This broke the geode-aes driver.
>
> So let's fix it by moving to the sync skcipher interface when allocating
> the fallback. At the same time, align with the generic API for ECB and
> CBC by rejecting inputs that are not a multiple of the AES block size.
>
> Fixes: 79c65d179a40e145 ("crypto: cbc - Convert to skcipher")
> Cc: <stable@vger.kernel.org> # v4.20+ ONLY
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> v2: pass dst and src scatterlist in the right order
> reject inputs that are not a multiple of the block size
>
> drivers/crypto/geode-aes.c | 57 +++++++++++---------
> drivers/crypto/geode-aes.h | 2 +-
> 2 files changed, 34 insertions(+), 25 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-19 13:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-05 9:11 [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback Ard Biesheuvel
2019-10-05 12:22 ` Florian Bezdeka
2019-10-08 11:36 ` Ard Biesheuvel
2020-01-19 10:02 ` Florian Bezdeka
2020-01-19 13:51 ` Ard Biesheuvel
2019-10-05 16:15 ` Gert Robben
2019-10-08 11:38 ` Ard Biesheuvel
2019-10-10 12:58 ` Herbert Xu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.