2017-02-27 01:03:43

by Marcelo Henrique Cerri

[permalink] [raw]
Subject: [PATCH 0/2] Propagate fallback bit for cbc and ctr

Hi Herbert,

These are similar changes for cbc and ctr as the one you did for xts. They
rely on the helper function you created.

I confirmed that for vmx-crypto those changes cause the driver to allocate
"cbc(aes-generic)" and "ctr(aes-generic)" as fallbacks instead of
"cbc(p8_aes)" and "ctr(p8_aes)".

If you are ok with those changes, I can convert the remaining templates.

Marcelo Henrique Cerri (2):
crypto: cbc - Propagate NEED_FALLBACK bit
crypto: ctr - Propagate NEED_FALLBACK bit

crypto/cbc.c | 20 ++++++++++++++------
crypto/ctr.c | 13 +++++++++++--
2 files changed, 25 insertions(+), 8 deletions(-)

--
2.7.4


2017-02-27 01:03:47

by Marcelo Henrique Cerri

[permalink] [raw]
Subject: [PATCH 1/2] crypto: cbc - Propagate NEED_FALLBACK bit

When requesting a fallback algorithm, we should propagate the
NEED_FALLBACK bit when search for the underlying algorithm.

This will prevents drivers from allocating unnecessary fallbacks that
are never called. For instance, currently the vmx-crypto driver will use
the following chain of calls when calling the fallback implementation:

p8_aes_cbc -> cbc(p8_aes) -> aes-generic

However p8_aes will always delegate its calls to aes-generic. With this
patch, p8_aes_cbc will be able to use cbc(aes-generic) directly as its
fallback. The same applies to aes_s390.

Signed-off-by: Marcelo Henrique Cerri <[email protected]>
---
crypto/cbc.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/crypto/cbc.c b/crypto/cbc.c
index bc160a3..7147842 100644
--- a/crypto/cbc.c
+++ b/crypto/cbc.c
@@ -108,24 +108,32 @@ static void crypto_cbc_free(struct skcipher_instance *inst)
static int crypto_cbc_create(struct crypto_template *tmpl, struct rtattr **tb)
{
struct skcipher_instance *inst;
+ struct crypto_attr_type *algt;
struct crypto_spawn *spawn;
struct crypto_alg *alg;
+ u32 mask;
int err;

err = crypto_check_attr_type(tb, CRYPTO_ALG_TYPE_SKCIPHER);
if (err)
return err;

+ algt = crypto_get_attr_type(tb);
+ if (IS_ERR(algt))
+ return PTR_ERR(algt);
+
+ mask = CRYPTO_ALG_TYPE_MASK |
+ crypto_requires_off(algt->type, algt->mask,
+ CRYPTO_ALG_NEED_FALLBACK);
+
+ alg = crypto_get_attr_alg(tb, CRYPTO_ALG_TYPE_CIPHER, mask);
+ if (IS_ERR(alg))
+ return PTR_ERR(alg);
+
inst = kzalloc(sizeof(*inst) + sizeof(*spawn), GFP_KERNEL);
if (!inst)
return -ENOMEM;

- alg = crypto_get_attr_alg(tb, CRYPTO_ALG_TYPE_CIPHER,
- CRYPTO_ALG_TYPE_MASK);
- err = PTR_ERR(alg);
- if (IS_ERR(alg))
- goto err_free_inst;
-
spawn = skcipher_instance_ctx(inst);
err = crypto_init_spawn(spawn, alg, skcipher_crypto_instance(inst),
CRYPTO_ALG_TYPE_MASK);
--
2.7.4

2017-02-27 01:03:19

by Marcelo Henrique Cerri

[permalink] [raw]
Subject: [PATCH 2/2] crypto: ctr - Propagate NEED_FALLBACK bit

When requesting a fallback algorithm, we should propagate the
NEED_FALLBACK bit when search for the underlying algorithm.

This will prevents drivers from allocating unnecessary fallbacks that
are never called. For instance, currently the vmx-crypto driver will use
the following chain of calls when calling the fallback implementation:

p8_aes_ctr -> ctr(p8_aes) -> aes-generic

However p8_aes will always delegate its calls to aes-generic. With this
patch, p8_aes_ctr will be able to use ctr(aes-generic) directly as its
fallback. The same applies to aes_s390.

Signed-off-by: Marcelo Henrique Cerri <[email protected]>
---
crypto/ctr.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/crypto/ctr.c b/crypto/ctr.c
index a4f4a89..3afe21a 100644
--- a/crypto/ctr.c
+++ b/crypto/ctr.c
@@ -181,15 +181,24 @@ static void crypto_ctr_exit_tfm(struct crypto_tfm *tfm)
static struct crypto_instance *crypto_ctr_alloc(struct rtattr **tb)
{
struct crypto_instance *inst;
+ struct crypto_attr_type *algt;
struct crypto_alg *alg;
+ u32 mask;
int err;

err = crypto_check_attr_type(tb, CRYPTO_ALG_TYPE_BLKCIPHER);
if (err)
return ERR_PTR(err);

- alg = crypto_attr_alg(tb[1], CRYPTO_ALG_TYPE_CIPHER,
- CRYPTO_ALG_TYPE_MASK);
+ algt = crypto_get_attr_type(tb);
+ if (IS_ERR(algt))
+ return PTR_ERR(algt);
+
+ mask = CRYPTO_ALG_TYPE_MASK |
+ crypto_requires_off(algt->type, algt->mask,
+ CRYPTO_ALG_NEED_FALLBACK);
+
+ alg = crypto_attr_alg(tb[1], CRYPTO_ALG_TYPE_CIPHER, mask);
if (IS_ERR(alg))
return ERR_CAST(alg);

--
2.7.4

2017-02-27 09:51:36

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: cbc - Propagate NEED_FALLBACK bit

On Sun, Feb 26, 2017 at 10:03:18PM -0300, Marcelo Henrique Cerri wrote:
> When requesting a fallback algorithm, we should propagate the
> NEED_FALLBACK bit when search for the underlying algorithm.
>
> This will prevents drivers from allocating unnecessary fallbacks that
> are never called. For instance, currently the vmx-crypto driver will use
> the following chain of calls when calling the fallback implementation:
>
> p8_aes_cbc -> cbc(p8_aes) -> aes-generic
>
> However p8_aes will always delegate its calls to aes-generic. With this
> patch, p8_aes_cbc will be able to use cbc(aes-generic) directly as its
> fallback. The same applies to aes_s390.
>
> Signed-off-by: Marcelo Henrique Cerri <[email protected]>
> ---
> crypto/cbc.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/crypto/cbc.c b/crypto/cbc.c
> index bc160a3..7147842 100644
> --- a/crypto/cbc.c
> +++ b/crypto/cbc.c
> @@ -108,24 +108,32 @@ static void crypto_cbc_free(struct skcipher_instance *inst)
> static int crypto_cbc_create(struct crypto_template *tmpl, struct rtattr **tb)
> {
> struct skcipher_instance *inst;
> + struct crypto_attr_type *algt;
> struct crypto_spawn *spawn;
> struct crypto_alg *alg;
> + u32 mask;
> int err;
>
> err = crypto_check_attr_type(tb, CRYPTO_ALG_TYPE_SKCIPHER);
> if (err)
> return err;
>
> + algt = crypto_get_attr_type(tb);
> + if (IS_ERR(algt))
> + return PTR_ERR(algt);
> +
> + mask = CRYPTO_ALG_TYPE_MASK |
> + crypto_requires_off(algt->type, algt->mask,
> + CRYPTO_ALG_NEED_FALLBACK);
> +
> + alg = crypto_get_attr_alg(tb, CRYPTO_ALG_TYPE_CIPHER, mask);
> + if (IS_ERR(alg))
> + return PTR_ERR(alg);
> +
> inst = kzalloc(sizeof(*inst) + sizeof(*spawn), GFP_KERNEL);
> if (!inst)
> return -ENOMEM;

You're leaking alg if the kzalloc of inst fails. Easiest fix
would be to do crypto_get_attr_alg after the kzalloc as is the
status quo.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt