Re: [PATCH v2 0/2] Propagate fallback bit for cbc and ctr

[Marcelo Cerri]

Hi Herbert,

Any thoughts on this?

On Mon, Feb 27, 2017 at 09:38:24AM -0300, Marcelo Henrique Cerri wrote:
> Hi Hebert,
> 
> For v2:
> 
> - fixed the memory leakage in cbc.
> - included crypto/algapi.h in crypto/cbc.c for crypto_requires_off();
> - ERR_CAST instead PTR_ERR in ctr.
> - Also propagated the fallback bit for rfc3686.
> 
> Marcelo Henrique Cerri (2):
>   crypto: cbc - Propagate NEED_FALLBACK bit
>   crypto: ctr - Propagate NEED_FALLBACK bit
> 
>  crypto/cbc.c | 15 +--
>  crypto/ctr.c | 23 ++-
>  2 files changed, 31 insertions(+), 7 deletions(-)
> 
> -- 
> 2.7.4
> 

-- 
Regards,
Marcelo



signature.asc
Description: PGP signature

Re: [PATCH v2 1/2] crypto: vmx - Use skcipher for cbc fallback

[Marcelo Cerri]

On Fri, Feb 24, 2017 at 11:23:54AM -0300, Paulo Flabiano Smorigo wrote:
> Signed-off-by: Paulo Flabiano Smorigo 
> ---
>  drivers/crypto/vmx/aes_cbc.c | 44 
> ++--
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
> index 94ad5c0..2bb5910 100644
> --- a/drivers/crypto/vmx/aes_cbc.c
> +++ b/drivers/crypto/vmx/aes_cbc.c
> @@ -27,11 +27,12 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "aesp8-ppc.h"
>  
>  struct p8_aes_cbc_ctx {
> - struct crypto_blkcipher *fallback;
> + struct crypto_skcipher *fallback;
>   struct aes_key enc_key;
>   struct aes_key dec_key;
>  };
> @@ -39,7 +40,7 @@ struct p8_aes_cbc_ctx {
>  static int p8_aes_cbc_init(struct crypto_tfm *tfm)
>  {
>   const char *alg;
> - struct crypto_blkcipher *fallback;
> + struct crypto_skcipher *fallback;
>   struct p8_aes_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
>  
>   if (!(alg = crypto_tfm_alg_name(tfm))) {
> @@ -47,8 +48,9 @@ static int p8_aes_cbc_init(struct crypto_tfm *tfm)
>   return -ENOENT;
>   }
>  
> - fallback =
> - crypto_alloc_blkcipher(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
> + fallback = crypto_alloc_skcipher(alg, 0,
> + CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK);
> +
>   if (IS_ERR(fallback)) {
>   printk(KERN_ERR
>  "Failed to allocate transformation for '%s': %ld\n",
> @@ -58,9 +60,9 @@ static int p8_aes_cbc_init(struct crypto_tfm *tfm)
>   printk(KERN_INFO "Using '%s' as fallback implementation.\n",
>  crypto_tfm_alg_driver_name((struct crypto_tfm *) fallback));

You need to update that to use crypto_skcipher_tfm(). The same is valid
for the xts patch.

>  
> - crypto_blkcipher_set_flags(
> + crypto_skcipher_set_flags(
>   fallback,
> - crypto_blkcipher_get_flags((struct crypto_blkcipher *)tfm));
> + crypto_skcipher_get_flags((struct crypto_skcipher *)tfm));
>   ctx->fallback = fallback;
>  
>   return 0;
> @@ -71,7 +73,7 @@ static void p8_aes_cbc_exit(struct crypto_tfm *tfm)
>   struct p8_aes_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
>  
>   if (ctx->fallback) {
> - crypto_free_blkcipher(ctx->fallback);
> + crypto_free_skcipher(ctx->fallback);
>   ctx->fallback = NULL;
>   }
>  }
> @@ -91,7 +93,7 @@ static int p8_aes_cbc_setkey(struct crypto_tfm *tfm, const 
> u8 *key,
>   pagefault_enable();
>   preempt_enable();
>  
> - ret += crypto_blkcipher_setkey(ctx->fallback, key, keylen);
> + ret += crypto_skcipher_setkey(ctx->fallback, key, keylen);
>   return ret;
>  }
>  
> @@ -103,15 +105,14 @@ static int p8_aes_cbc_encrypt(struct blkcipher_desc 
> *desc,
>   struct blkcipher_walk walk;
>   struct p8_aes_cbc_ctx *ctx =
>   crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
> - struct blkcipher_desc fallback_desc = {
> - .tfm = ctx->fallback,
> - .info = desc->info,
> - .flags = desc->flags
> - };
>  
>   if (in_interrupt()) {
> - ret = crypto_blkcipher_encrypt(_desc, dst, src,
> -nbytes);
> + SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
> + skcipher_request_set_tfm(req, ctx->fallback);
> + skcipher_request_set_callback(req, desc->flags, NULL, NULL);
> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
> + ret = crypto_skcipher_encrypt(req);
> + skcipher_request_zero(req);
>   } else {
>   preempt_disable();
>   pagefault_disable();
> @@ -144,15 +145,14 @@ static int p8_aes_cbc_decrypt(struct blkcipher_desc 
> *desc,
>   struct blkcipher_walk walk;
>   struct p8_aes_cbc_ctx *ctx =
>   crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
> - struct blkcipher_desc fallback_desc = {
> - .tfm = ctx->fallback,
> - .info = desc->info,
> - .flags = desc->flags
> - };
>  
>   if (in_interrupt()) {
> - ret = crypto_blkcipher_decrypt(_desc, dst, src,
> -nbytes);
> + SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
> + skcipher_request_set_tfm(req, ctx->fallback);
> + skcipher_request_set_callback(req, desc->flags, NULL, NULL);
> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
> + ret = crypto_skcipher_decrypt(req);
> + skcipher_request_zero(req);
>   } else {
>   preempt_disable();
>   pagefault_disable();
> -- 
> 2.7.4
> 

-- 
Regards,
Marcelo



signature.asc
Description: PGP signature

Re: crypto: hang in crypto_larval_lookup

[Marcelo Cerri]

On Sat, Feb 25, 2017 at 11:17:07PM +0800, Herbert Xu wrote:
> On Fri, Feb 24, 2017 at 08:44:00PM -0300, Marcelo Cerri wrote:
> >
> > This is probably caused by the way that the xts template is handling the
> > underline algorithm selection.
> 
> Good catch.  I think the bigger issue here is that when requesting
> for an XTS that doesn't need a fallback we shouldn't be using an
> underlying ECB that needs one.  So this patch should fix the hang.

Yeah, I agree. This should work as long as the module aliases are
correct, which is enough.

Other templates will not trigger the same error since they don't have to
try more than one underlying algorithm. But I think this is still
desirable for the remaining templates to avoid a long chain of unused
fallbacks as in the example I gave in my previous email.

Probably a helper function to return the correct mask might be useful
for readability and to avoid duplicate code.

> 
> Thanks,
> 
> ---8<---
> Subject: crypto: xts - Propagate NEED_FALLBACK bit
> 
> When we're used as a fallback algorithm, we should propagate
> the NEED_FALLBACK bit when searching for the underlying ECB mode.
> 
> This just happens to fix a hang too because otherwise the search
> may end up loading the same module that triggered this XTS creation.
> 
> Fixes: f1c131b45410 ("crypto: xts - Convert to skcipher")
> Reported-by: Harald Freudenberger <fre...@linux.vnet.ibm.com>
> Signed-off-by: Herbert Xu <herb...@gondor.apana.org.au>
> 
> diff --git a/crypto/xts.c b/crypto/xts.c
> index 410a2e2..2066161 100644
> --- a/crypto/xts.c
> +++ b/crypto/xts.c
> @@ -463,6 +463,7 @@ static int create(struct crypto_template *tmpl, struct 
> rtattr **tb)
>   struct xts_instance_ctx *ctx;
>   struct skcipher_alg *alg;
>   const char *cipher_name;
> + u32 mask;
>   int err;
>  
>   algt = crypto_get_attr_type(tb);
> @@ -483,18 +484,19 @@ static int create(struct crypto_template *tmpl, struct 
> rtattr **tb)
>   ctx = skcipher_instance_ctx(inst);
>  
>   crypto_set_skcipher_spawn(>spawn, skcipher_crypto_instance(inst));
> - err = crypto_grab_skcipher(>spawn, cipher_name, 0,
> -crypto_requires_sync(algt->type,
> - algt->mask));
> +
> + mask = crypto_requires_sync(algt->type, algt->mask) |
> +((algt->type ^ CRYPTO_ALG_NEED_FALLBACK) & algt->mask &
> + CRYPTO_ALG_NEED_FALLBACK);
> +
> + err = crypto_grab_skcipher(>spawn, cipher_name, 0, mask);
>   if (err == -ENOENT) {
>   err = -ENAMETOOLONG;
>   if (snprintf(ctx->name, CRYPTO_MAX_ALG_NAME, "ecb(%s)",
>cipher_name) >= CRYPTO_MAX_ALG_NAME)
>   goto err_free_inst;
>  
> - err = crypto_grab_skcipher(>spawn, ctx->name, 0,
> -crypto_requires_sync(algt->type,
> - algt->mask));
> + err = crypto_grab_skcipher(>spawn, ctx->name, 0, mask);
>   }
>  
>   if (err)
> -- 
> Email: Herbert Xu <herb...@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

-- 
Regards,
Marcelo



signature.asc
Description: PGP signature

Re: [PATCH 1/2] crypto: vmx - Use skcipher for cbc fallback

[Marcelo Cerri]

It makes sense. Thanks for the clarification, Herbert.

One more question: are you planning to convert the ctr template to
skcipher?

-- 
Regards,
Marcelo

On Thu, Feb 23, 2017 at 07:21:56PM +0800, Herbert Xu wrote:
> Marcelo Cerri <marcelo.ce...@canonical.com> wrote:
> >
> > I noticed you used a similar approach in arch/s390/crypto/aes_s390.c
> > (commit 64e2680).
> > 
> > How do you ensure the skcipher operation will not be asynchronous? 
> 
> You need to set the bit CRYPTO_ALG_ASYNC in the mask field when
> allocating the algorithm to ensure that it's synchronous.
> 
> Cheers,
> -- 
> Email: Herbert Xu <herb...@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt



signature.asc
Description: PGP signature

Re: [PATCH 1/2] crypto: vmx - Use skcipher for cbc fallback

[Marcelo Cerri]

Hi Hebert,

On Wed, Feb 22, 2017 at 04:18:19PM -0300, Marcelo Cerri wrote:
> Hi Paulo.
> 
> On Wed, Feb 22, 2017 at 03:00:15PM -0300, Paulo Flabiano Smorigo wrote:
> >  
> > if (in_interrupt()) {
> > -   ret = crypto_blkcipher_encrypt(_desc, dst, src,
> > -  nbytes);
> > +   SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
> > +   skcipher_request_set_tfm(req, ctx->fallback);
> > +   skcipher_request_set_callback(req, desc->flags, NULL, NULL);
> > +   skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
> > +   ret = crypto_skcipher_encrypt(req);
> > +   skcipher_request_zero(req);
> 
> Probably you have to wait for the completion here before proceeding.
> 
> Check Documentation/crypto/api-samples.rst
> 

I noticed you used a similar approach in arch/s390/crypto/aes_s390.c
(commit 64e2680).

How do you ensure the skcipher operation will not be asynchronous? 

-- 
Regards,
Marcelo



signature.asc
Description: PGP signature

Re: [PATCH 2/2] crypto: vmx - Use skcipher for xts fallback

[Marcelo Cerri]

In addition to cbc comments:

On Wed, Feb 22, 2017 at 03:00:45PM -0300, Paulo Flabiano Smorigo wrote:
> Signed-off-by: Paulo Flabiano Smorigo 
> ---
>  drivers/crypto/vmx/aes_xts.c | 32 ++--
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
> index 24353ec3..a8245e1 100644
> --- a/drivers/crypto/vmx/aes_xts.c
> +++ b/drivers/crypto/vmx/aes_xts.c
> @@ -28,11 +28,12 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "aesp8-ppc.h"
>  
>  struct p8_aes_xts_ctx {
> - struct crypto_blkcipher *fallback;
> + struct crypto_skcipher *fallback;
>   struct aes_key enc_key;
>   struct aes_key dec_key;
>   struct aes_key tweak_key;
> @@ -41,7 +42,7 @@ struct p8_aes_xts_ctx {
>  static int p8_aes_xts_init(struct crypto_tfm *tfm)
>  {
>   const char *alg;
> - struct crypto_blkcipher *fallback;
> + struct crypto_skcipher *fallback;
>   struct p8_aes_xts_ctx *ctx = crypto_tfm_ctx(tfm);
>  
>   if (!(alg = crypto_tfm_alg_name(tfm))) {
> @@ -50,7 +51,7 @@ static int p8_aes_xts_init(struct crypto_tfm *tfm)
>   }
>  
>   fallback =
> - crypto_alloc_blkcipher(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
> + crypto_alloc_skcipher(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
>   if (IS_ERR(fallback)) {
>   printk(KERN_ERR
>   "Failed to allocate transformation for '%s': %ld\n",
> @@ -60,9 +61,9 @@ static int p8_aes_xts_init(struct crypto_tfm *tfm)
>   printk(KERN_INFO "Using '%s' as fallback implementation.\n",
>   crypto_tfm_alg_driver_name((struct crypto_tfm *) fallback));
>  
> - crypto_blkcipher_set_flags(
> + crypto_skcipher_set_flags(
>   fallback,
> - crypto_blkcipher_get_flags((struct crypto_blkcipher *)tfm));
> + crypto_skcipher_get_flags((struct crypto_skcipher *)tfm));
>   ctx->fallback = fallback;
>  
>   return 0;
> @@ -73,7 +74,7 @@ static void p8_aes_xts_exit(struct crypto_tfm *tfm)
>   struct p8_aes_xts_ctx *ctx = crypto_tfm_ctx(tfm);
>  
>   if (ctx->fallback) {
> - crypto_free_blkcipher(ctx->fallback);
> + crypto_free_skcipher(ctx->fallback);
>   ctx->fallback = NULL;
>   }
>  }
> @@ -98,7 +99,7 @@ static int p8_aes_xts_setkey(struct crypto_tfm *tfm, const 
> u8 *key,
>   pagefault_enable();
>   preempt_enable();
>  
> - ret += crypto_blkcipher_setkey(ctx->fallback, key, keylen);
> + ret += crypto_skcipher_setkey(ctx->fallback, key, keylen);
>   return ret;
>  }
>  
> @@ -113,15 +114,18 @@ static int p8_aes_xts_crypt(struct blkcipher_desc *desc,
>   struct blkcipher_walk walk;
>   struct p8_aes_xts_ctx *ctx =
>   crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
> - struct blkcipher_desc fallback_desc = {
> - .tfm = ctx->fallback,
> - .info = desc->info,
> - .flags = desc->flags
> - };
>  
>   if (in_interrupt()) {
> - ret = enc ? crypto_blkcipher_encrypt(_desc, dst, src, 
> nbytes) :
> -crypto_blkcipher_decrypt(_desc, dst, 
> src, nbytes);
> + SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
> + skcipher_request_set_tfm(req, ctx->fallback);
> + skcipher_request_set_callback(req, desc->flags, NULL, NULL);
> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
> + ret = crypto_skcipher_encrypt(req);

You probably don't want this crypto_skcipher_encrypt call.


> + if (enc)
> + crypto_skcipher_encrypt(req);
> + else
> + crypto_skcipher_decrypt(req);

And you should check the return values here.

> + skcipher_request_zero(req);
>   } else {
>   preempt_disable();
>   pagefault_disable();
> -- 
> 2.9.3
> 

-- 
Regards,
Marcelo



signature.asc
Description: PGP signature

Re: [PATCH 1/2] crypto: vmx - Use skcipher for cbc fallback

[Marcelo Cerri]

Hi Paulo.

On Wed, Feb 22, 2017 at 03:00:15PM -0300, Paulo Flabiano Smorigo wrote:
> Signed-off-by: Paulo Flabiano Smorigo 
> ---
>  drivers/crypto/vmx/aes_cbc.c | 41 -
>  1 file changed, 20 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
> index 94ad5c0..5aa70997 100644
> --- a/drivers/crypto/vmx/aes_cbc.c
> +++ b/drivers/crypto/vmx/aes_cbc.c
> @@ -27,11 +27,12 @@
>  #include 
>  #include 
>  #include 
> +#include 

Isn't crypto/skcipher.h enough?

>  
>  #include "aesp8-ppc.h"
>  
>  struct p8_aes_cbc_ctx {
> - struct crypto_blkcipher *fallback;
> + struct crypto_skcipher *fallback;
>   struct aes_key enc_key;
>   struct aes_key dec_key;
>  };
> @@ -39,7 +40,7 @@ struct p8_aes_cbc_ctx {
>  static int p8_aes_cbc_init(struct crypto_tfm *tfm)
>  {
>   const char *alg;
> - struct crypto_blkcipher *fallback;
> + struct crypto_skcipher *fallback;
>   struct p8_aes_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
>  
>   if (!(alg = crypto_tfm_alg_name(tfm))) {
> @@ -48,7 +49,7 @@ static int p8_aes_cbc_init(struct crypto_tfm *tfm)
>   }
>  
>   fallback =
> - crypto_alloc_blkcipher(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
> + crypto_alloc_skcipher(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
>   if (IS_ERR(fallback)) {
>   printk(KERN_ERR
>  "Failed to allocate transformation for '%s': %ld\n",
> @@ -58,9 +59,9 @@ static int p8_aes_cbc_init(struct crypto_tfm *tfm)
>   printk(KERN_INFO "Using '%s' as fallback implementation.\n",
>  crypto_tfm_alg_driver_name((struct crypto_tfm *) fallback));
>  
> - crypto_blkcipher_set_flags(
> + crypto_skcipher_set_flags(
>   fallback,
> - crypto_blkcipher_get_flags((struct crypto_blkcipher *)tfm));
> + crypto_skcipher_get_flags((struct crypto_skcipher *)tfm));
>   ctx->fallback = fallback;
>  
>   return 0;
> @@ -71,7 +72,7 @@ static void p8_aes_cbc_exit(struct crypto_tfm *tfm)
>   struct p8_aes_cbc_ctx *ctx = crypto_tfm_ctx(tfm);
>  
>   if (ctx->fallback) {
> - crypto_free_blkcipher(ctx->fallback);
> + crypto_free_skcipher(ctx->fallback);
>   ctx->fallback = NULL;
>   }
>  }
> @@ -91,7 +92,7 @@ static int p8_aes_cbc_setkey(struct crypto_tfm *tfm, const 
> u8 *key,
>   pagefault_enable();
>   preempt_enable();
>  
> - ret += crypto_blkcipher_setkey(ctx->fallback, key, keylen);
> + ret += crypto_skcipher_setkey(ctx->fallback, key, keylen);
>   return ret;
>  }
>  
> @@ -103,15 +104,14 @@ static int p8_aes_cbc_encrypt(struct blkcipher_desc 
> *desc,
>   struct blkcipher_walk walk;
>   struct p8_aes_cbc_ctx *ctx =
>   crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
> - struct blkcipher_desc fallback_desc = {
> - .tfm = ctx->fallback,
> - .info = desc->info,
> - .flags = desc->flags
> - };
>  
>   if (in_interrupt()) {
> - ret = crypto_blkcipher_encrypt(_desc, dst, src,
> -nbytes);
> + SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
> + skcipher_request_set_tfm(req, ctx->fallback);
> + skcipher_request_set_callback(req, desc->flags, NULL, NULL);
> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
> + ret = crypto_skcipher_encrypt(req);
> + skcipher_request_zero(req);

Probably you have to wait for the completion here before proceeding.

Check Documentation/crypto/api-samples.rst


>   } else {
>   preempt_disable();
>   pagefault_disable();
> @@ -144,15 +144,14 @@ static int p8_aes_cbc_decrypt(struct blkcipher_desc 
> *desc,
>   struct blkcipher_walk walk;
>   struct p8_aes_cbc_ctx *ctx =
>   crypto_tfm_ctx(crypto_blkcipher_tfm(desc->tfm));
> - struct blkcipher_desc fallback_desc = {
> - .tfm = ctx->fallback,
> - .info = desc->info,
> - .flags = desc->flags
> - };
>  
>   if (in_interrupt()) {
> - ret = crypto_blkcipher_decrypt(_desc, dst, src,
> -nbytes);
> + SKCIPHER_REQUEST_ON_STACK(req, ctx->fallback);
> + skcipher_request_set_tfm(req, ctx->fallback);
> + skcipher_request_set_callback(req, desc->flags, NULL, NULL);
> + skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
> + ret = crypto_skcipher_decrypt(req);
> + skcipher_request_zero(req);

Same here.


>   } else {
>   preempt_disable();
>   pagefault_disable();
> -- 
> 2.9.3
> 

-- 
Regards,
Marcelo



signature.asc
Description: PGP signature

Re: crypto: xts: regression in 4.10

[Marcelo Cerri]

What XTS implementations do you have available on /proc/crypto after the
error?

Some drivers that allocate fallback implementations using the older API
started to fail after the generic templates were converted to skcipher.


On Wed, Feb 22, 2017 at 12:17:17AM +0100, Nicolas Porcel wrote:
> Hello,
> 
> I am using aes-xts-plain64 with LUKS headers to crypt the root. In 4.10,
> the partition cannot be opened and I have the following errors when
> booting:
> 
> device-mapper: table: 253:0: crypt: Error allocating crypto tfm
> device-mapper: ioctl: error adding target to table
> device-mapper: reload ioctl on  failed: No such file or directory
> Failed to setup dm-crypt key mapping for device /dev/mmcblk0p2
> Check that the kernel supports aes-xts-plain64 cipher (check syslog for 
> more info)
> 
> I found that this commit is responsible for the regression (reverting it
> solves the problem):
> 
> > commit f1c131b45410a202eb45cc55980a7a9e4e4b4f40
> > Author: Herbert Xu 
> > Date:   Tue Nov 22 20:08:19 2016 +0800
> > 
> > crypto: xts - Convert to skcipher
> 
> Some precision: I am using the vanilla kernel source for 4.10. The aes,
> xts and dm-crypt modules are directly compiled in the kernel and not as
> modules. I also had the same problem with kernel 4.10-rc*.
> 
> Is it a known issue? I found 1 related email with no answer on the
> dm-crypt mailing. If this is a regression, I can start digging, although
> any guidance would be greatly appreciated.
> 
> Thank you in advance,
> 
> Best regards,
> 
> -- 
> Nicolas Porcel

-- 
Regards,
Marcelo



signature.asc
Description: PGP signature

Re: [PATCH 10/16] crypto: testmgr - Do not test internal algorithms

[Marcelo Cerri]

I tested this patch and it's working fine.

-- 
Regards,
Marcelo

On Wed, Nov 02, 2016 at 07:19:12AM +0800, Herbert Xu wrote:
> Currently we manually filter out internal algorithms using a list
> in testmgr.  This is dangerous as internal algorithms cannot be
> safely used even by testmgr.  This patch ensures that they're never
> processed by testmgr at all.
> 
> This patch also removes an obsolete bypass for nivciphers which
> no longer exist.
> 
> Signed-off-by: Herbert Xu 
> ---
> 
>  crypto/algboss.c |8 --
>  crypto/testmgr.c |  153 
> +++
>  2 files changed, 11 insertions(+), 150 deletions(-)
> 
> diff --git a/crypto/algboss.c b/crypto/algboss.c
> index 6e39d9c..ccb85e1 100644
> --- a/crypto/algboss.c
> +++ b/crypto/algboss.c
> @@ -247,12 +247,8 @@ static int cryptomgr_schedule_test(struct crypto_alg 
> *alg)
>   memcpy(param->alg, alg->cra_name, sizeof(param->alg));
>   type = alg->cra_flags;
>  
> - /* This piece of crap needs to disappear into per-type test hooks. */
> - if (!((type ^ CRYPTO_ALG_TYPE_BLKCIPHER) &
> -   CRYPTO_ALG_TYPE_BLKCIPHER_MASK) && !(type & CRYPTO_ALG_GENIV) &&
> - ((alg->cra_flags & CRYPTO_ALG_TYPE_MASK) ==
> -  CRYPTO_ALG_TYPE_BLKCIPHER ? alg->cra_blkcipher.ivsize :
> -  alg->cra_ablkcipher.ivsize))
> + /* Do not test internal algorithms. */
> + if (type & CRYPTO_ALG_INTERNAL)
>   type |= CRYPTO_ALG_TESTED;
>  
>   param->type = type;
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index ded50b6..6ac4696 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -1625,7 +1625,7 @@ static int alg_test_aead(const struct alg_test_desc 
> *desc, const char *driver,
>   struct crypto_aead *tfm;
>   int err = 0;
>  
> - tfm = crypto_alloc_aead(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_aead(driver, type, mask);
>   if (IS_ERR(tfm)) {
>   printk(KERN_ERR "alg: aead: Failed to load transform for %s: "
>  "%ld\n", driver, PTR_ERR(tfm));
> @@ -1654,7 +1654,7 @@ static int alg_test_cipher(const struct alg_test_desc 
> *desc,
>   struct crypto_cipher *tfm;
>   int err = 0;
>  
> - tfm = crypto_alloc_cipher(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_cipher(driver, type, mask);
>   if (IS_ERR(tfm)) {
>   printk(KERN_ERR "alg: cipher: Failed to load transform for "
>  "%s: %ld\n", driver, PTR_ERR(tfm));
> @@ -1683,7 +1683,7 @@ static int alg_test_skcipher(const struct alg_test_desc 
> *desc,
>   struct crypto_skcipher *tfm;
>   int err = 0;
>  
> - tfm = crypto_alloc_skcipher(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_skcipher(driver, type, mask);
>   if (IS_ERR(tfm)) {
>   printk(KERN_ERR "alg: skcipher: Failed to load transform for "
>  "%s: %ld\n", driver, PTR_ERR(tfm));
> @@ -1750,7 +1750,7 @@ static int alg_test_hash(const struct alg_test_desc 
> *desc, const char *driver,
>   struct crypto_ahash *tfm;
>   int err;
>  
> - tfm = crypto_alloc_ahash(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_ahash(driver, type, mask);
>   if (IS_ERR(tfm)) {
>   printk(KERN_ERR "alg: hash: Failed to load transform for %s: "
>  "%ld\n", driver, PTR_ERR(tfm));
> @@ -1778,7 +1778,7 @@ static int alg_test_crc32c(const struct alg_test_desc 
> *desc,
>   if (err)
>   goto out;
>  
> - tfm = crypto_alloc_shash(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_shash(driver, type, mask);
>   if (IS_ERR(tfm)) {
>   printk(KERN_ERR "alg: crc32c: Failed to load transform for %s: "
>  "%ld\n", driver, PTR_ERR(tfm));
> @@ -1820,7 +1820,7 @@ static int alg_test_cprng(const struct alg_test_desc 
> *desc, const char *driver,
>   struct crypto_rng *rng;
>   int err;
>  
> - rng = crypto_alloc_rng(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + rng = crypto_alloc_rng(driver, type, mask);
>   if (IS_ERR(rng)) {
>   printk(KERN_ERR "alg: cprng: Failed to load transform for %s: "
>  "%ld\n", driver, PTR_ERR(rng));
> @@ -1847,7 +1847,7 @@ static int drbg_cavs_test(struct drbg_testvec *test, 
> int pr,
>   if (!buf)
>   return -ENOMEM;
>  
> - drng = crypto_alloc_rng(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + drng = crypto_alloc_rng(driver, type, mask);
>   if (IS_ERR(drng)) {
>   printk(KERN_ERR "alg: drbg: could not allocate DRNG handle for "
>  "%s\n", driver);
> @@ -2041,7 +2041,7 @@ static int alg_test_kpp(const struct alg_test_desc 
> *desc, const char *driver,
>   struct crypto_kpp *tfm;
>   int err = 0;
>  
> - tfm = 

[PATCH 2/3] crypto: testmgr - Add missing tests for internal sha256-mb implementation

[Marcelo Cerri]

Add null tests for the internal algorithm to avoid errors when running
in FIPS mode.

Signed-off-by: Marcelo Cerri <marcelo.ce...@canonical.com>
---
 crypto/testmgr.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index d999373..58f903d 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -2309,6 +2309,10 @@ static const struct alg_test_desc alg_test_descs[] = {
.test = alg_test_null,
.fips_allowed = 1,
}, {
+   .alg = "__intel_sha256-mb",
+   .test = alg_test_null,
+   .fips_allowed = 1,
+   }, {
.alg = "ansi_cprng",
.test = alg_test_cprng,
.suite = {
@@ -3758,6 +3762,10 @@ static const struct alg_test_desc alg_test_descs[] = {
.test = alg_test_null,
.fips_allowed = 1,
}, {
+   .alg = "mcryptd(__intel_sha256-mb)",
+   .test = alg_test_null,
+   .fips_allowed = 1,
+   }, {
.alg = "md4",
.test = alg_test_hash,
.suite = {
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] crypto: testmgr - Add missing tests for internal sha512-mb implementation

[Marcelo Cerri]

Add null tests for the internal algorithm to avoid errors when running
in FIPS mode.

Signed-off-by: Marcelo Cerri <marcelo.ce...@canonical.com>
---
 crypto/testmgr.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 58f903d..cfafd24 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -2313,6 +2313,10 @@ static const struct alg_test_desc alg_test_descs[] = {
.test = alg_test_null,
.fips_allowed = 1,
}, {
+   .alg = "__intel_sha512-mb",
+   .test = alg_test_null,
+   .fips_allowed = 1,
+   }, {
.alg = "ansi_cprng",
.test = alg_test_cprng,
.suite = {
@@ -3766,6 +3770,10 @@ static const struct alg_test_desc alg_test_descs[] = {
.test = alg_test_null,
.fips_allowed = 1,
}, {
+   .alg = "mcryptd(__intel_sha512-mb)",
+   .test = alg_test_null,
+   .fips_allowed = 1,
+   }, {
.alg = "md4",
.test = alg_test_hash,
.suite = {
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] crypto: testmgr - Add missing tests for internal sha*-mb implementations

[Marcelo Cerri]

This series adds null tests for all sha*-mb internal algorithms so they can
be used in FIPS mode without further problems.

Since they are 3 separated modules I decided to use a separated commit for
each one.

Marcelo Cerri (3):
  crypto: testmgr - Add missing tests for internal sha1-mb
implementation
  crypto: testmgr - Add missing tests for internal sha256-mb
implementation
  crypto: testmgr - Add missing tests for internal sha512-mb
implementation

 crypto/testmgr.c | 24 
 1 file changed, 24 insertions(+)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] crypto: testmgr - Add missing tests for internal sha1-mb implementation

[Marcelo Cerri]

Add null tests for the internal algorithm to avoid errors when running
in FIPS mode.

Signed-off-by: Marcelo Cerri <marcelo.ce...@canonical.com>
---
 crypto/testmgr.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index ded50b6..d999373 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -2305,6 +2305,10 @@ static const struct alg_test_desc alg_test_descs[] = {
.test = alg_test_null,
.fips_allowed = 1,
}, {
+   .alg = "__intel_sha1-mb",
+   .test = alg_test_null,
+   .fips_allowed = 1,
+   }, {
.alg = "ansi_cprng",
.test = alg_test_cprng,
.suite = {
@@ -3750,6 +3754,10 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}
}, {
+   .alg = "mcryptd(__intel_sha1-mb)",
+   .test = alg_test_null,
+   .fips_allowed = 1,
+   }, {
.alg = "md4",
.test = alg_test_hash,
.suite = {
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: sha1-powerpc: little-endian support

[Marcelo Cerri]

Hi Michael,

On Ubuntu, CRYPTO_MANAGER_DISABLE_TESTS is set by default. So I had to
disable this config in order to make sha1-powerpc fail in the crypto API
tests. However, even with tests disabled, any usage of sha1-powerpc
should result in incorrect results.

-- 
Regards,
Marcelo

On Tue, Oct 04, 2016 at 05:23:16PM +1100, Michael Ellerman wrote:
> Marcelo Cerri <marcelo.ce...@canonical.com> writes:
> 
> > [ Unknown signature status ]
> > On Wed, Sep 28, 2016 at 09:20:15PM +0800, Herbert Xu wrote:
> >> On Wed, Sep 28, 2016 at 10:15:51AM -0300, Marcelo Cerri wrote:
> >> > Hi Herbert,
> >> > 
> >> > Any thoughts on this one?
> >> 
> >> Can this patch wait until the next merge window? On the broken
> >> platforms it should just fail the self-test, right?
> >
> > Yes. It fails on any LE platform (including Ubuntu and RHEL 7.1).
> 
> How are you testing this? I thought I was running the crypto tests but
> I've never seen this fail.
> 
> cheers


signature.asc
Description: PGP signature

Re: [PATCH 0/3] Fix crypto/vmx/p8_ghash memory corruption

[Marcelo Cerri]

Hi Herbert,

Sorry for bothering you. I noticed you included two of the patches in
the crypto-2.6 repository and the remaining one in cryptodev-2.6. Is
that right? I thought all 3 patches would be included in the cruptodev
repository.

-- 
Regards,
Marcelo

On Wed, Sep 28, 2016 at 01:42:08PM -0300, Marcelo Cerri wrote:
> This series fixes the memory corruption found by Jan Stancek in 4.8-rc7. The
> problem however also affects previous versions of the driver.
> 
> Marcelo Cerri (3):
>   crypto: ghash-generic - move common definitions to a new header file
>   crypto: vmx - Fix memory corruption caused by p8_ghash
>   crypto: vmx - Ensure ghash-generic is enabled
> 
>  crypto/ghash-generic.c | 13 +
>  drivers/crypto/vmx/Kconfig |  2 +-
>  drivers/crypto/vmx/ghash.c | 31 ---
>  include/crypto/ghash.h | 23 +++
>  4 files changed, 41 insertions(+), 28 deletions(-)
>  create mode 100644 include/crypto/ghash.h
> 
> -- 
> 2.7.4
> 


signature.asc
Description: PGP signature

[PATCH 3/3] crypto: vmx - Ensure ghash-generic is enabled

[Marcelo Cerri]

Add CRYPTO_GHASH as a dependency for vmx_crypto since p8_ghash uses it
as the fallback implementation.

Signed-off-by: Marcelo Cerri <marcelo.ce...@canonical.com>
---
 drivers/crypto/vmx/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/vmx/Kconfig b/drivers/crypto/vmx/Kconfig
index a83ead1..c21b09a 100644
--- a/drivers/crypto/vmx/Kconfig
+++ b/drivers/crypto/vmx/Kconfig
@@ -1,6 +1,6 @@
 config CRYPTO_DEV_VMX_ENCRYPT
tristate "Encryption acceleration support on P8 CPU"
-   depends on CRYPTO_DEV_VMX
+   depends on CRYPTO_DEV_VMX || CRYPTO_GHASH
default m
help
  Support for VMX cryptographic acceleration instructions on Power8 CPU.
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] Fix crypto/vmx/p8_ghash memory corruption

[Marcelo Cerri]

This series fixes the memory corruption found by Jan Stancek in 4.8-rc7. The
problem however also affects previous versions of the driver.

Marcelo Cerri (3):
  crypto: ghash-generic - move common definitions to a new header file
  crypto: vmx - Fix memory corruption caused by p8_ghash
  crypto: vmx - Ensure ghash-generic is enabled

 crypto/ghash-generic.c | 13 +
 drivers/crypto/vmx/Kconfig |  2 +-
 drivers/crypto/vmx/ghash.c | 31 ---
 include/crypto/ghash.h | 23 +++
 4 files changed, 41 insertions(+), 28 deletions(-)
 create mode 100644 include/crypto/ghash.h

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: sha1-powerpc: little-endian support

[Marcelo Cerri]

Hi Herbert,

Any thoughts on this one?

-- 
Regards,
Marcelo

On Fri, Sep 23, 2016 at 04:31:56PM -0300, Marcelo Cerri wrote:
> The driver does not handle endianness properly when loading the input
> data.
> 
> Signed-off-by: Marcelo Cerri <marcelo.ce...@canonical.com>
> ---
>  arch/powerpc/crypto/sha1-powerpc-asm.S | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/crypto/sha1-powerpc-asm.S 
> b/arch/powerpc/crypto/sha1-powerpc-asm.S
> index 125e165..82ddc9b 100644
> --- a/arch/powerpc/crypto/sha1-powerpc-asm.S
> +++ b/arch/powerpc/crypto/sha1-powerpc-asm.S
> @@ -7,6 +7,15 @@
>  #include 
>  #include 
>  
> +#ifdef __BIG_ENDIAN__
> +#define LWZ(rt, d, ra)   \
> + lwz rt,d(ra)
> +#else
> +#define LWZ(rt, d, ra)   \
> + li  rt,d;   \
> + lwbrx   rt,rt,ra
> +#endif
> +
>  /*
>   * We roll the registers for T, A, B, C, D, E around on each
>   * iteration; T on iteration t is A on iteration t+1, and so on.
> @@ -23,7 +32,7 @@
>  #define W(t) (((t)%16)+16)
>  
>  #define LOADW(t) \
> - lwz W(t),(t)*4(r4)
> + LWZ(W(t),(t)*4,r4)
>  
>  #define STEPD0_LOAD(t)   \
>   andcr0,RD(t),RB(t); \
> @@ -33,7 +42,7 @@
>   add r0,RE(t),r15;   \
>   add RT(t),RT(t),r6; \
>   add r14,r0,W(t);\
> - lwz W((t)+4),((t)+4)*4(r4); \
> + LWZ(W((t)+4),((t)+4)*4,r4); \
>   rotlwi  RB(t),RB(t),30; \
>   add RT(t),RT(t),r14
>  
> -- 
> 2.7.4
> 


signature.asc
Description: PGP signature

Re: [bug] crypto/vmx/p8_ghash memory corruption in 4.8-rc7

[Marcelo Cerri]

Hi Herbert,

On Wed, Sep 28, 2016 at 10:45:49AM +0800, Herbert Xu wrote:
> On Tue, Sep 27, 2016 at 04:46:44PM -0300, Marcelo Cerri wrote:
> >
> > Can you check if the problem occurs with this patch?
>
> In light of the fact that padlock-sha is the correct example
> to follow, you only need to add one line to the init_tfm fucntion
> to update the descsize based on that of the fallback.
>

Not sure if I'm missing something here but p8_ghash already does that:

 56 static int p8_ghash_init_tfm(struct crypto_tfm *tfm)
 57 {
 ...
 83 shash_tfm->descsize = sizeof(struct p8_ghash_desc_ctx)
 84 + crypto_shash_descsize(fallback);
 85
 86 return 0;
 87 }

I think the problem here is that crypto_ahash_statesize uses the
statesize value (that is initialized with the descsize value from
shash_alg) instead of using the value from the tfm that was updated.

For padlock_sha1, it uses the value from alg->statesize since it defines
import and export functions. It doesn't make much difference if it uses
the value from descsize or statesize here, what really matter is that
it uses the value defined in struct shash_alg and not from the tfm.

The big difference between p8_ghash and padlock_sha1 is that
padlock_sha1 defines alg->statesize as sizeof(struct sha1_state), which
is the descsize value used by sha1_generic. This probably works but
it's also wrong because the padlock_sha1 driver does not ensures that
sha1_generic is always used.

So, one solution is to hardcode ghash-generic as the fallback algorithm
and update the descsize direct in its shash_alg structure. There's only
one problem with that. ghash-generic desc type (struct ghash_desc_ctx)
is not available for vmx_crypto at compile type, the type is defined
directly in crypto/ghash-generic.c. That's the reason I added a function
to get the fallback desc size at runtime in the patch I wrote as a prove
of concept.

--
Regards,
Marcelo


signature.asc
Description: PGP signature

Re: [bug] crypto/vmx/p8_ghash memory corruption in 4.8-rc7

[Marcelo Cerri]

Jan, 

Can you check if the problem occurs with this patch?

---
 drivers/crypto/vmx/ghash.c | 28 +---
 drivers/crypto/vmx/vmx.c   |  9 +
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/vmx/ghash.c b/drivers/crypto/vmx/ghash.c
index 6c999cb0..033aba1 100644
--- a/drivers/crypto/vmx/ghash.c
+++ b/drivers/crypto/vmx/ghash.c
@@ -36,6 +36,8 @@
 #define GHASH_DIGEST_SIZE (16)
 #define GHASH_KEY_LEN (16)
 
+#define GHASH_FALLBACK_ALG "ghash-generic"
+
 void gcm_init_p8(u128 htable[16], const u64 Xi[2]);
 void gcm_gmult_p8(u64 Xi[2], const u128 htable[16]);
 void gcm_ghash_p8(u64 Xi[2], const u128 htable[16],
@@ -53,18 +55,26 @@ struct p8_ghash_desc_ctx {
struct shash_desc fallback_desc;
 };
 
+int p8_ghash_fallback_descsize(void)
+{
+   int descsize;
+   struct crypto_shash *fallback;
+   fallback = crypto_alloc_shash(GHASH_FALLBACK_ALG, 0,
+ CRYPTO_ALG_NEED_FALLBACK);
+   if (IS_ERR(fallback)) {
+   return PTR_ERR(fallback);
+   }
+   descsize = crypto_shash_descsize(fallback);
+   crypto_free_shash(fallback);
+   return descsize;
+}
+
 static int p8_ghash_init_tfm(struct crypto_tfm *tfm)
 {
-   const char *alg;
+   const char *alg = GHASH_FALLBACK_ALG;
struct crypto_shash *fallback;
-   struct crypto_shash *shash_tfm = __crypto_shash_cast(tfm);
struct p8_ghash_ctx *ctx = crypto_tfm_ctx(tfm);
 
-   if (!(alg = crypto_tfm_alg_name(tfm))) {
-   printk(KERN_ERR "Failed to get algorithm name.\n");
-   return -ENOENT;
-   }
-
fallback = crypto_alloc_shash(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(fallback)) {
printk(KERN_ERR
@@ -79,10 +89,6 @@ static int p8_ghash_init_tfm(struct crypto_tfm *tfm)
   crypto_shash_get_flags((struct crypto_shash
   *) tfm));
ctx->fallback = fallback;
-
-   shash_tfm->descsize = sizeof(struct p8_ghash_desc_ctx)
-   + crypto_shash_descsize(fallback);
-
return 0;
 }
 
diff --git a/drivers/crypto/vmx/vmx.c b/drivers/crypto/vmx/vmx.c
index 31a98dc..8a51149 100644
--- a/drivers/crypto/vmx/vmx.c
+++ b/drivers/crypto/vmx/vmx.c
@@ -28,6 +28,8 @@
 #include 
 #include 
 
+int p8_ghash_fallback_descsize(void);
+
 extern struct shash_alg p8_ghash_alg;
 extern struct crypto_alg p8_aes_alg;
 extern struct crypto_alg p8_aes_cbc_alg;
@@ -45,6 +47,7 @@ int __init p8_init(void)
 {
int ret = 0;
struct crypto_alg **alg_it;
+   int ghash_descsize;
 
for (alg_it = algs; *alg_it; alg_it++) {
ret = crypto_register_alg(*alg_it);
@@ -59,6 +62,12 @@ int __init p8_init(void)
if (ret)
return ret;
 
+   ghash_descsize = p8_ghash_fallback_descsize();
+   if (ghash_descsize < 0) {
+   printk(KERN_ERR "Cannot get descsize for p8_ghash fallback\n");
+   return ghash_descsize;
+   }
+   p8_ghash_alg.descsize += ghash_descsize;
ret = crypto_register_shash(_ghash_alg);
if (ret) {
for (alg_it = algs; *alg_it; alg_it++)
-- 
2.7.4



signature.asc
Description: PGP signature

Re: [bug] crypto/vmx/p8_ghash memory corruption in 4.8-rc7

[Marcelo Cerri]

Herbert,

Wouldn't be enough to provide a pair of import/export functions as the
padlock-sha driver does?

-- 
Regards,
Marcelo

On Mon, Sep 26, 2016 at 10:59:34PM +0800, Herbert Xu wrote:
> On Fri, Sep 23, 2016 at 08:22:27PM -0400, Jan Stancek wrote:
> >
> > This seems to directly correspond with:
> >   p8_ghash_alg.descsize = sizeof(struct p8_ghash_desc_ctx) == 56
> >   shash_tfm->descsize = sizeof(struct p8_ghash_desc_ctx) + 
> > crypto_shash_descsize(fallback) == 56 + 20
> > where 20 is presumably coming from "ghash_alg.descsize".
> > 
> > My gut feeling was that these 2 should match, but I'd love to hear
> > what crypto people think.
> 
> Indeed.  The vmx driver is broken.  It is allocating a fallback
> but is not providing any space for the state of the fallback.
> 
> Unfortunately our interface doesn't really provide a way to provide
> the state size dynamically.  So what I'd suggest is to fix the
> fallback to the generic ghash implementation and export its state
> size like we do for md5/sha.
> 
> Cheers,
> -- 
> Email: Herbert Xu 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


signature.asc
Description: PGP signature

[PATCH] crypto: sha1-powerpc: little-endian support

[Marcelo Cerri]

The driver does not handle endianness properly when loading the input
data.

Signed-off-by: Marcelo Cerri <marcelo.ce...@canonical.com>
---
 arch/powerpc/crypto/sha1-powerpc-asm.S | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/crypto/sha1-powerpc-asm.S 
b/arch/powerpc/crypto/sha1-powerpc-asm.S
index 125e165..82ddc9b 100644
--- a/arch/powerpc/crypto/sha1-powerpc-asm.S
+++ b/arch/powerpc/crypto/sha1-powerpc-asm.S
@@ -7,6 +7,15 @@
 #include 
 #include 
 
+#ifdef __BIG_ENDIAN__
+#define LWZ(rt, d, ra) \
+   lwz rt,d(ra)
+#else
+#define LWZ(rt, d, ra) \
+   li  rt,d;   \
+   lwbrx   rt,rt,ra
+#endif
+
 /*
  * We roll the registers for T, A, B, C, D, E around on each
  * iteration; T on iteration t is A on iteration t+1, and so on.
@@ -23,7 +32,7 @@
 #define W(t)   (((t)%16)+16)
 
 #define LOADW(t)   \
-   lwz W(t),(t)*4(r4)
+   LWZ(W(t),(t)*4,r4)
 
 #define STEPD0_LOAD(t) \
andcr0,RD(t),RB(t); \
@@ -33,7 +42,7 @@
add r0,RE(t),r15;   \
add RT(t),RT(t),r6; \
add r14,r0,W(t);\
-   lwz W((t)+4),((t)+4)*4(r4); \
+   LWZ(W((t)+4),((t)+4)*4,r4); \
rotlwi  RB(t),RB(t),30; \
add RT(t),RT(t),r14
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: crypto: GCM API usage

[Marcelo Cerri]

On Thu, Oct 03, 2013 at 08:03:45AM +0200, tobias.pol...@fau.de wrote:
  I haven't used the IV generation facility of the Crypto API, but it
  seems to be very straightforward although there's no documentation
  about that.
 
  You should use aead_givcrypt_set_callback(), aead_givcrypt_set_assoc()
  and aead_givcrypt_set_crypt() as you would use the regular aead
  functions, that includes that you have to provide a buffer with length
  equals to the algorithm block size for the IV. And then you should call
  aead_givcrypt_set_giv() passing a counter and another IV buffer.
 
  The difference between the two IV buffers that you have to provide to
  aead_givcrypt_set_crypt() and aead_givcrypt_set_giv() is that the first
  one will be updated by the algorithm during the encryption of each block
  and the second one will contain the generated IV that you will have to
  use to decrypt data.
 
  The last step is to call crypto_aead_givencrypt() as you would call
  crypto_aead_encrypt().
 
 We discovered those functions, yet the only way we found how to use them was
 to use one of the ipsec modes, e.g.:
   crypto_alloc_aead(rfc4106(gcm(aes)), 0, 0)
 
 Is this the only way this API should be used, or is there some high level
 interface to use iv generators like seqiv?

In order to use IV generation, the targeted algorithm must specify a
generation method and the plain GCM implementation actually doesn't do
it.

Both rfc4106 and rfc4543 (gmac) say that the implementation can use any
IV generation method that meets the uniqueness requirement without
coordinating with the receiver. I think that is the reason that only
these two variations of GCM explicitly define an IV generation method.

If I'm not wrong the Crypto API was first designed to support ipsec
needs, so it makes sense that it simplify things for it. However, I
don't see any reason for GCM itself not have a default IV generation
method, since regular and giv interfaces can be used.

So you should keep explicitly handling the IV generation or maybe submit
a patch adding a default geniv for GCM. I think Herbert can give us more
information about the history behind the geniv support and correct me
if I said anything wrong.

 
 Thank you for your help,
   Dominik Paulus and Tobias Polzer
 

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: crypto: GCM API usage

[Marcelo Cerri]

On Mon, Sep 16, 2013 at 08:34:11PM +0200, Dominik Paulus wrote:
 Hi,
 
 On Mon, Sep 16, 2013 at 12:58:40PM +0200, 
 dominik.d.pau...@studium.uni-erlangen.de wrote:
  We are currently trying to add encryption support to the usbip kernel
  driver. Unfortunately, there is almost no documentation for the kernel
  crypto API. So far, we couldn't figure out how to use the GCM encryption
  mode in the kernel. There seems to be infrastructure for IV generation
  in place (e.g. seqiv.c, the geniv stuff and the RFC 4106 implementation),
  but no code directly using it.
  
  What's the recommended way to use the IV generators with a high-level
  API?
 
 Sorry, that mail probably got a bit too short. To explain our problem a bit
 more: We are currently using a 64-bit counter to generate IVs. As the
 keys are randomly generated for each session and thus never reused,
 that's probably a not too bad idea (if it is, please tell us why ;)),
 assuming this counter is never going to overflow. We pass the IVs
 directly to aead_request_set_crypt for each message. This currently
 works quite fine.

It's usual the use of random IVs but I don't think that any known
pattern (as a counter) in the IV generation should cause any reduction
in the algorithm strength (and if it does, it's probably a weakness in
the algorithm itself). The only thing that should be avoided for sure is
the reuse of the same IV.

 
 However, we would expect that IV generation is at least partially handled
 by the crypto API. As I said, there seems to be infrastructure for that,
 that abstracts the sequence number quite nicely. The seqiv generator
 seems to provide a high-level interface to the AEAD crypto, including an 
 abstraction for the sequence number generation. However, due to the lack
 of documentation and/or reference code using the API, we couldn't find
 out how to use it yet.

I haven't used the IV generation facility of the Crypto API, but it
seems to be very straightforward although there's no documentation
about that.

You should use aead_givcrypt_set_callback(), aead_givcrypt_set_assoc()
and aead_givcrypt_set_crypt() as you would use the regular aead
functions, that includes that you have to provide a buffer with length
equals to the algorithm block size for the IV. And then you should call
aead_givcrypt_set_giv() passing a counter and another IV buffer.

The difference between the two IV buffers that you have to provide to
aead_givcrypt_set_crypt() and aead_givcrypt_set_giv() is that the first
one will be updated by the algorithm during the encryption of each block
and the second one will contain the generated IV that you will have to
use to decrypt data.

The last step is to call crypto_aead_givencrypt() as you would call
crypto_aead_encrypt().

Under the cover the Crypto API will generate a random salt in the first
use of each request. This salt will be xor'd with the given counter to
create the new IV. That has an advantage over a simple count, that
usually will start with the same value every time the system is
rebooted.

 
 Any help on this would be appreciated. If we feel competent enough to do 
 so after finishing this project, we would also volunteer to extend the
 introduction in Documentation/crypto/api-intro.txt a bit.
 

That is probably a very good idea! I also want to improve the
documentation as soon I have some time to do it :)

 Regards,
 Tobias Polzer and Dominik Paulus
 --
 To unsubscribe from this list: send the line unsubscribe linux-crypto in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto_memcmp: add constant-time memcmp

[Marcelo Cerri]

The discussion that Daniel pointed out has another interesting point
regarding the function name. I don't think it's a good idea to name it
crypto_memcpy since it doesn't have behavior the same way as strcmp.

Florian suggested in the thread names such crypto_mem_equal, which I
think fits better here.

Regards,
Marcelo

On Tue, Sep 10, 2013 at 08:57:55PM +0200, Daniel Borkmann wrote:
 On 09/10/2013 08:38 PM, James Yonan wrote:
 When comparing MAC hashes, AEAD authentication tags, or other hash
 values in the context of authentication or integrity checking, it
 is important not to leak timing information to a potential attacker.
 
 Bytewise memory comparisons (such as memcmp) are usually optimized so
 that they return a nonzero value as soon as a mismatch is found.
 This early-return behavior can leak timing information, allowing an
 attacker to iteratively guess the correct result.
 
 This patch adds a new method crypto_memcmp that has the same prototype
 as standard memcmp, but that compares strings of the same length in
 roughly constant time (cache misses could change the timing, but
 since they don't reveal information about the content of the strings
 being compared, they are effectively benign).  Note that crypto_memcmp
 (unlike memcmp) can only be used to test for equality or inequality,
 NOT greater-than or less-than.  This is not an issue for its use-cases
 within the Crypto API.
 
 I tried to locate all of the places in the Crypto API where memcmp was
 being used for authentication or integrity checking, and convert them
 over to crypto_memcmp.
 
 crypto_memcmp is declared noinline and placed in its own source file
 because a very smart compiler (or LTO) might notice that the return
 value is always compared against zero/nonzero, and might then
 reintroduce the same early-return optimization that we are trying to
 avoid.
 
 There was a similar patch posted some time ago [1] on lkml, where
 Florian (CC) made a good point in [2] that future compiler optimizations
 could short circuit on this. This issue should probably be addressed in
 such a patch here as well.
 
  [1] https://lkml.org/lkml/2013/2/10/131
  [2] https://lkml.org/lkml/2013/2/11/381
 
 Signed-off-by: James Yonan ja...@openvpn.net
 ---
   crypto/Makefile |  2 +-
   crypto/asymmetric_keys/rsa.c|  5 +++--
   crypto/authenc.c|  7 ---
   crypto/authencesn.c |  9 +
   crypto/ccm.c|  5 +++--
   crypto/crypto_memcmp.c  | 31 
  +++
   crypto/gcm.c|  3 ++-
   include/crypto/internal/crypto_memcmp.h | 20 
   8 files changed, 69 insertions(+), 13 deletions(-)
   create mode 100644 crypto/crypto_memcmp.c
   create mode 100644 include/crypto/internal/crypto_memcmp.h
 
 diff --git a/crypto/Makefile b/crypto/Makefile
 index 2ba0df2..39a574d 100644
 --- a/crypto/Makefile
 +++ b/crypto/Makefile
 @@ -3,7 +3,7 @@
   #
 
   obj-$(CONFIG_CRYPTO) += crypto.o
 -crypto-y := api.o cipher.o compress.o
 +crypto-y := api.o cipher.o compress.o crypto_memcmp.o
 
   obj-$(CONFIG_CRYPTO_WORKQUEUE) += crypto_wq.o
 
 diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
 index 4a6a069..4f9a250 100644
 --- a/crypto/asymmetric_keys/rsa.c
 +++ b/crypto/asymmetric_keys/rsa.c
 @@ -13,6 +13,7 @@
   #include linux/module.h
   #include linux/kernel.h
   #include linux/slab.h
 +#include crypto/internal/crypto_memcmp.h
   #include public_key.h
 
   MODULE_LICENSE(GPL);
 @@ -189,12 +190,12 @@ static int RSA_verify(const u8 *H, const u8 *EM, 
 size_t k, size_t hash_size,
  }
  }
 
 -if (memcmp(asn1_template, EM + T_offset, asn1_size) != 0) {
 +if (crypto_memcmp(asn1_template, EM + T_offset, asn1_size) != 0) {
  kleave( = -EBADMSG [EM[T] ASN.1 mismatch]);
  return -EBADMSG;
  }
 
 -if (memcmp(H, EM + T_offset + asn1_size, hash_size) != 0) {
 +if (crypto_memcmp(H, EM + T_offset + asn1_size, hash_size) != 0) {
  kleave( = -EKEYREJECTED [EM[T] hash mismatch]);
  return -EKEYREJECTED;
  }
 diff --git a/crypto/authenc.c b/crypto/authenc.c
 index ffce19d..82ca98f 100644
 --- a/crypto/authenc.c
 +++ b/crypto/authenc.c
 @@ -13,6 +13,7 @@
   #include crypto/aead.h
   #include crypto/internal/hash.h
   #include crypto/internal/skcipher.h
 +#include crypto/internal/crypto_memcmp.h
   #include crypto/authenc.h
   #include crypto/scatterwalk.h
   #include linux/err.h
 @@ -188,7 +189,7 @@ static void authenc_verify_ahash_update_done(struct 
 crypto_async_request *areq,
  scatterwalk_map_and_copy(ihash, areq_ctx-sg, areq_ctx-cryptlen,
   authsize, 0);
 
 -err = memcmp(ihash, ahreq-result, authsize) ? -EBADMSG : 0;
 +err = crypto_memcmp(ihash, ahreq-result, authsize) ? -EBADMSG : 0;
  if (err)
  goto out;
 
 @@ -227,7 +228,7 @@ 

Re: [PATCH 03/10] crypto: nx - fix limits to sg lists for AES-CBC

[Marcelo Cerri]

On Thu, Aug 29, 2013 at 02:42:22PM +1000, Herbert Xu wrote:
 On Fri, Aug 23, 2013 at 05:01:07PM -0300, Marcelo Cerri wrote:
  This patch updates the nx-aes-cbc implementation to perform several
  hyper calls if needed in order to always respect the length limits for
  scatter/gather lists.
  
  Two different limits are considered:
  
   - ibm,max-sg-len: maximum number of bytes of each scatter/gather
 list.
  
   - ibm,max-sync-cop:
  - The total number of bytes that a scatter/gather list can hold.
  - The maximum number of elements that a scatter/gather list can have.
  
  Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
  Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
 
 This patch does not apply against the current cryptodev tree.
 
 Please regenerate your pathces.

Sorry for this. I'm sending a v2 series without conflicts.

 
 Thanks,
 -- 
 Email: Herbert Xu herb...@gondor.apana.org.au
 Home Page: http://gondor.apana.org.au/~herbert/
 PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
 --
 To unsubscribe from this list: send the line unsubscribe linux-crypto in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 02/10] crypto: nx - fix limits to sg lists for AES-ECB

[Marcelo Cerri]

This patch updates the nx-aes-ecb implementation to perform several
hyper calls if needed in order to always respect the length limits for
scatter/gather lists.

Two different limits are considered:

 - ibm,max-sg-len: maximum number of bytes of each scatter/gather
   list.

 - ibm,max-sync-cop:
- The total number of bytes that a scatter/gather list can hold.
- The maximum number of elements that a scatter/gather list can have.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-ecb.c | 48 ++
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-ecb.c b/drivers/crypto/nx/nx-aes-ecb.c
index fe0d803..85a8d23 100644
--- a/drivers/crypto/nx/nx-aes-ecb.c
+++ b/drivers/crypto/nx/nx-aes-ecb.c
@@ -71,37 +71,49 @@ static int ecb_aes_nx_crypt(struct blkcipher_desc *desc,
struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc-tfm);
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
unsigned long irq_flags;
+   unsigned int processed = 0, to_process;
+   u32 max_sg_len;
int rc;
 
spin_lock_irqsave(nx_ctx-lock, irq_flags);
 
-   if (nbytes  nx_ctx-ap-databytelen) {
-   rc = -EINVAL;
-   goto out;
-   }
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+  nx_ctx-ap-sglen);
 
if (enc)
NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
else
NX_CPB_FDM(csbcpb) = ~NX_FDM_ENDE_ENCRYPT;
 
-   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0, NULL);
-   if (rc)
-   goto out;
+   do {
+   to_process = min_t(u64, nbytes - processed,
+  nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+  NX_PAGE_SIZE * (max_sg_len - 1));
+   to_process = to_process  ~(AES_BLOCK_SIZE - 1);
 
-   if (!nx_ctx-op.inlen || !nx_ctx-op.outlen) {
-   rc = -EINVAL;
-   goto out;
-   }
+   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, to_process,
+   processed, NULL);
+   if (rc)
+   goto out;
 
-   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
-  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
-   if (rc)
-   goto out;
+   if (!nx_ctx-op.inlen || !nx_ctx-op.outlen) {
+   rc = -EINVAL;
+   goto out;
+   }
+
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
+  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   goto out;
+
+   atomic_inc((nx_ctx-stats-aes_ops));
+   atomic64_add(csbcpb-csb.processed_byte_count,
+(nx_ctx-stats-aes_bytes));
+
+   processed += to_process;
+   } while (processed  nbytes);
 
-   atomic_inc((nx_ctx-stats-aes_ops));
-   atomic64_add(csbcpb-csb.processed_byte_count,
-(nx_ctx-stats-aes_bytes));
 out:
spin_unlock_irqrestore(nx_ctx-lock, irq_flags);
return rc;
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 07/10] crypto: nx - fix limits to sg lists for AES-CCM

[Marcelo Cerri]

From: Fionnuala Gunter f...@linux.vnet.ibm.com

This patch updates the NX driver to perform several hyper calls when necessary
so that the length limits of scatter/gather lists are respected.

Reviewed-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Fionnuala Gunter f...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-ccm.c | 297 +
 1 file changed, 215 insertions(+), 82 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-ccm.c b/drivers/crypto/nx/nx-aes-ccm.c
index 666a35b..5ecd4c2 100644
--- a/drivers/crypto/nx/nx-aes-ccm.c
+++ b/drivers/crypto/nx/nx-aes-ccm.c
@@ -179,13 +179,26 @@ static int generate_pat(u8   *iv,
struct nx_sg *nx_insg = nx_ctx-in_sg;
struct nx_sg *nx_outsg = nx_ctx-out_sg;
unsigned int iauth_len = 0;
-   struct vio_pfo_op *op = NULL;
u8 tmp[16], *b1 = NULL, *b0 = NULL, *result = NULL;
int rc;
 
/* zero the ctr value */
memset(iv + 15 - iv[0], 0, iv[0] + 1);
 
+   /* page 78 of nx_wb.pdf has,
+* Note: RFC3610 allows the AAD data to be up to 2^64 -1 bytes
+* in length. If a full message is used, the AES CCA implementation
+* restricts the maximum AAD length to 2^32 -1 bytes.
+* If partial messages are used, the implementation supports
+* 2^64 -1 bytes maximum AAD length.
+*
+* However, in the cryptoapi's aead_request structure,
+* assoclen is an unsigned int, thus it cannot hold a length
+* value greater than 2^32 - 1.
+* Thus the AAD is further constrained by this and is never
+* greater than 2^32.
+*/
+
if (!req-assoclen) {
b0 = nx_ctx-csbcpb-cpb.aes_ccm.in_pat_or_b0;
} else if (req-assoclen = 14) {
@@ -195,7 +208,46 @@ static int generate_pat(u8   *iv,
b0 = nx_ctx-csbcpb-cpb.aes_ccm.in_pat_or_b0;
b1 = nx_ctx-priv.ccm.iauth_tag;
iauth_len = req-assoclen;
+   } else if (req-assoclen = 65280) {
+   /* if associated data is less than (2^16 - 2^8), we construct
+* B1 differently and feed in the associated data to a CCA
+* operation */
+   b0 = nx_ctx-csbcpb_aead-cpb.aes_cca.b0;
+   b1 = nx_ctx-csbcpb_aead-cpb.aes_cca.b1;
+   iauth_len = 14;
+   } else {
+   b0 = nx_ctx-csbcpb_aead-cpb.aes_cca.b0;
+   b1 = nx_ctx-csbcpb_aead-cpb.aes_cca.b1;
+   iauth_len = 10;
+   }
 
+   /* generate B0 */
+   rc = generate_b0(iv, req-assoclen, authsize, nbytes, b0);
+   if (rc)
+   return rc;
+
+   /* generate B1:
+* add control info for associated data
+* RFC 3610 and NIST Special Publication 800-38C
+*/
+   if (b1) {
+   memset(b1, 0, 16);
+   if (req-assoclen = 65280) {
+   *(u16 *)b1 = (u16)req-assoclen;
+   scatterwalk_map_and_copy(b1 + 2, req-assoc, 0,
+iauth_len, SCATTERWALK_FROM_SG);
+   } else {
+   *(u16 *)b1 = (u16)(0xfffe);
+   *(u32 *)b1[2] = (u32)req-assoclen;
+   scatterwalk_map_and_copy(b1 + 6, req-assoc, 0,
+iauth_len, SCATTERWALK_FROM_SG);
+   }
+   }
+
+   /* now copy any remaining AAD to scatterlist and call nx... */
+   if (!req-assoclen) {
+   return rc;
+   } else if (req-assoclen = 14) {
nx_insg = nx_build_sg_list(nx_insg, b1, 16, nx_ctx-ap-sglen);
nx_outsg = nx_build_sg_list(nx_outsg, tmp, 16,
nx_ctx-ap-sglen);
@@ -210,56 +262,74 @@ static int generate_pat(u8   *iv,
NX_CPB_FDM(nx_ctx-csbcpb) |= NX_FDM_ENDE_ENCRYPT;
NX_CPB_FDM(nx_ctx-csbcpb) |= NX_FDM_INTERMEDIATE;
 
-   op = nx_ctx-op;
result = nx_ctx-csbcpb-cpb.aes_ccm.out_pat_or_mac;
-   } else if (req-assoclen = 65280) {
-   /* if associated data is less than (2^16 - 2^8), we construct
-* B1 differently and feed in the associated data to a CCA
-* operation */
-   b0 = nx_ctx-csbcpb_aead-cpb.aes_cca.b0;
-   b1 = nx_ctx-csbcpb_aead-cpb.aes_cca.b1;
-   iauth_len = 14;
-
-   /* remaining assoc data must have scatterlist built for it */
-   nx_insg = nx_walk_and_build(nx_insg, nx_ctx-ap-sglen,
-   req-assoc, iauth_len,
-   req-assoclen - iauth_len);
-   nx_ctx-op_aead.inlen = (nx_ctx-in_sg - nx_insg) *
+
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op

[PATCH v2 10/10] crypto: nx - fix SHA-2 for chunks bigger than block size

[Marcelo Cerri]

Each call to the co-processor, with exception of the last call, needs to
send data that is multiple of block size. As consequence, any remaining
data is kept in the internal NX context.

This patch fixes a bug in the driver that causes it to save incorrect
data into the context when data is bigger than the block size.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-sha256.c | 2 +-
 drivers/crypto/nx/nx-sha512.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/nx/nx-sha256.c b/drivers/crypto/nx/nx-sha256.c
index 6547a71..da0b24a 100644
--- a/drivers/crypto/nx/nx-sha256.c
+++ b/drivers/crypto/nx/nx-sha256.c
@@ -129,7 +129,7 @@ static int nx_sha256_update(struct shash_desc *desc, const 
u8 *data,
NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
 
total -= to_process;
-   data += to_process;
+   data += to_process - sctx-count;
sctx-count = 0;
in_sg = nx_ctx-in_sg;
} while (leftover = SHA256_BLOCK_SIZE);
diff --git a/drivers/crypto/nx/nx-sha512.c b/drivers/crypto/nx/nx-sha512.c
index 236e6af..4ae5b0f 100644
--- a/drivers/crypto/nx/nx-sha512.c
+++ b/drivers/crypto/nx/nx-sha512.c
@@ -131,7 +131,7 @@ static int nx_sha512_update(struct shash_desc *desc, const 
u8 *data,
NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
 
total -= to_process;
-   data += to_process;
+   data += to_process - sctx-count[0];
sctx-count[0] = 0;
in_sg = nx_ctx-in_sg;
} while (leftover = SHA512_BLOCK_SIZE);
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 08/10] crypto: nx - fix XCBC for zero length messages

[Marcelo Cerri]

The NX XCBC implementation doesn't support zero length messages and
because of that NX is currently returning a hard-coded hash for zero
length messages. However this approach is incorrect since the hash value
also depends on which key is used.

This patch removes the hard-coded hash and replace it with an
implementation based on the RFC 3566 using ECB.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-xcbc.c | 84 +
 1 file changed, 77 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-xcbc.c b/drivers/crypto/nx/nx-aes-xcbc.c
index 1a5d9e3..03c4bf5 100644
--- a/drivers/crypto/nx/nx-aes-xcbc.c
+++ b/drivers/crypto/nx/nx-aes-xcbc.c
@@ -56,6 +56,77 @@ static int nx_xcbc_set_key(struct crypto_shash *desc,
return 0;
 }
 
+/*
+ * Based on RFC 3566, for a zero-length message:
+ *
+ * n = 1
+ * K1 = E(K, 0x01010101010101010101010101010101)
+ * K3 = E(K, 0x03030303030303030303030303030303)
+ * E[0] = 0x
+ * M[1] = 0x8000 (0 length message with padding)
+ * E[1] = (K1, M[1] ^ E[0] ^ K3)
+ * Tag = M[1]
+ */
+static int nx_xcbc_empty(struct shash_desc *desc, u8 *out)
+{
+   struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(desc-tfm-base);
+   struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
+   struct nx_sg *in_sg, *out_sg;
+   u8 keys[2][AES_BLOCK_SIZE];
+   u8 key[32];
+   int rc = 0;
+
+   /* Change to ECB mode */
+   csbcpb-cpb.hdr.mode = NX_MODE_AES_ECB;
+   memcpy(key, csbcpb-cpb.aes_xcbc.key, AES_BLOCK_SIZE);
+   memcpy(csbcpb-cpb.aes_ecb.key, key, AES_BLOCK_SIZE);
+   NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
+
+   /* K1 and K3 base patterns */
+   memset(keys[0], 0x01, sizeof(keys[0]));
+   memset(keys[1], 0x03, sizeof(keys[1]));
+
+   /* Generate K1 and K3 encrypting the patterns */
+   in_sg = nx_build_sg_list(nx_ctx-in_sg, (u8 *) keys, sizeof(keys),
+nx_ctx-ap-sglen);
+   out_sg = nx_build_sg_list(nx_ctx-out_sg, (u8 *) keys, sizeof(keys),
+ nx_ctx-ap-sglen);
+   nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) * sizeof(struct nx_sg);
+   nx_ctx-op.outlen = (nx_ctx-out_sg - out_sg) * sizeof(struct nx_sg);
+
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
+  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   goto out;
+   atomic_inc((nx_ctx-stats-aes_ops));
+
+   /* XOr K3 with the padding for a 0 length message */
+   keys[1][0] ^= 0x80;
+
+   /* Encrypt the final result */
+   memcpy(csbcpb-cpb.aes_ecb.key, keys[0], AES_BLOCK_SIZE);
+   in_sg = nx_build_sg_list(nx_ctx-in_sg, (u8 *) keys[1], sizeof(keys[1]),
+nx_ctx-ap-sglen);
+   out_sg = nx_build_sg_list(nx_ctx-out_sg, out, AES_BLOCK_SIZE,
+ nx_ctx-ap-sglen);
+   nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) * sizeof(struct nx_sg);
+   nx_ctx-op.outlen = (nx_ctx-out_sg - out_sg) * sizeof(struct nx_sg);
+
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
+  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   goto out;
+   atomic_inc((nx_ctx-stats-aes_ops));
+
+out:
+   /* Restore XCBC mode */
+   csbcpb-cpb.hdr.mode = NX_MODE_AES_XCBC_MAC;
+   memcpy(csbcpb-cpb.aes_xcbc.key, key, AES_BLOCK_SIZE);
+   NX_CPB_FDM(csbcpb) = ~NX_FDM_ENDE_ENCRYPT;
+
+   return rc;
+}
+
 static int nx_xcbc_init(struct shash_desc *desc)
 {
struct xcbc_state *sctx = shash_desc_ctx(desc);
@@ -201,13 +272,12 @@ static int nx_xcbc_final(struct shash_desc *desc, u8 *out)
memcpy(csbcpb-cpb.aes_xcbc.cv,
   csbcpb-cpb.aes_xcbc.out_cv_mac, AES_BLOCK_SIZE);
} else if (sctx-count == 0) {
-   /* we've never seen an update, so this is a 0 byte op. The
-* hardware cannot handle a 0 byte op, so just copy out the
-* known 0 byte result. This is cheaper than allocating a
-* software context to do a 0 byte op */
-   u8 data[] = { 0x75, 0xf0, 0x25, 0x1d, 0x52, 0x8a, 0xc0, 0x1c,
- 0x45, 0x73, 0xdf, 0xd5, 0x84, 0xd7, 0x9f, 0x29 };
-   memcpy(out, data, sizeof(data));
+   /*
+* we've never seen an update, so this is a 0 byte op. The
+* hardware cannot handle a 0 byte op, so just ECB to
+* generate the hash.
+*/
+   rc = nx_xcbc_empty(desc, out);
goto out;
}
 
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 03/10] crypto: nx - fix limits to sg lists for AES-CBC

[Marcelo Cerri]

This patch updates the nx-aes-cbc implementation to perform several
hyper calls if needed in order to always respect the length limits for
scatter/gather lists.

Two different limits are considered:

 - ibm,max-sg-len: maximum number of bytes of each scatter/gather
   list.

 - ibm,max-sync-cop:
- The total number of bytes that a scatter/gather list can hold.
- The maximum number of elements that a scatter/gather list can have.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-cbc.c | 50 +-
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c
index a9e76c6..cc00b52 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -71,39 +71,49 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc *desc,
struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc-tfm);
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
unsigned long irq_flags;
+   unsigned int processed = 0, to_process;
+   u32 max_sg_len;
int rc;
 
spin_lock_irqsave(nx_ctx-lock, irq_flags);
 
-   if (nbytes  nx_ctx-ap-databytelen) {
-   rc = -EINVAL;
-   goto out;
-   }
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+  nx_ctx-ap-sglen);
 
if (enc)
NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
else
NX_CPB_FDM(csbcpb) = ~NX_FDM_ENDE_ENCRYPT;
 
-   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0,
-  csbcpb-cpb.aes_cbc.iv);
-   if (rc)
-   goto out;
+   do {
+   to_process = min_t(u64, nbytes - processed,
+  nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+  NX_PAGE_SIZE * (max_sg_len - 1));
+   to_process = to_process  ~(AES_BLOCK_SIZE - 1);
 
-   if (!nx_ctx-op.inlen || !nx_ctx-op.outlen) {
-   rc = -EINVAL;
-   goto out;
-   }
+   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, to_process,
+  processed, csbcpb-cpb.aes_cbc.iv);
+   if (rc)
+   goto out;
 
-   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
-  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
-   if (rc)
-   goto out;
+   if (!nx_ctx-op.inlen || !nx_ctx-op.outlen) {
+   rc = -EINVAL;
+   goto out;
+   }
 
-   memcpy(desc-info, csbcpb-cpb.aes_cbc.cv, AES_BLOCK_SIZE);
-   atomic_inc((nx_ctx-stats-aes_ops));
-   atomic64_add(csbcpb-csb.processed_byte_count,
-(nx_ctx-stats-aes_bytes));
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
+  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   goto out;
+
+   memcpy(desc-info, csbcpb-cpb.aes_cbc.cv, AES_BLOCK_SIZE);
+   atomic_inc((nx_ctx-stats-aes_ops));
+   atomic64_add(csbcpb-csb.processed_byte_count,
+(nx_ctx-stats-aes_bytes));
+
+   processed += to_process;
+   } while (processed  nbytes);
 out:
spin_unlock_irqrestore(nx_ctx-lock, irq_flags);
return rc;
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 06/10] crypto: nx - fix limits to sg lists for AES-XCBC

[Marcelo Cerri]

From: Fionnuala Gunter f...@linux.vnet.ibm.com

This patch updates the NX driver to perform several hyper calls when necessary
so that the length limits of scatter/gather lists are respected.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Reviewed-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
Signed-off-by: Fionnuala Gunter f...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-xcbc.c | 107 +++-
 1 file changed, 63 insertions(+), 44 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-xcbc.c b/drivers/crypto/nx/nx-aes-xcbc.c
index 658da0f..1a5d9e3 100644
--- a/drivers/crypto/nx/nx-aes-xcbc.c
+++ b/drivers/crypto/nx/nx-aes-xcbc.c
@@ -88,78 +88,97 @@ static int nx_xcbc_update(struct shash_desc *desc,
struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(desc-tfm-base);
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
struct nx_sg *in_sg;
-   u32 to_process, leftover;
+   u32 to_process, leftover, total;
+   u32 max_sg_len;
unsigned long irq_flags;
int rc = 0;
 
spin_lock_irqsave(nx_ctx-lock, irq_flags);
 
-   if (NX_CPB_FDM(csbcpb)  NX_FDM_CONTINUATION) {
-   /* we've hit the nx chip previously and we're updating again,
-* so copy over the partial digest */
-   memcpy(csbcpb-cpb.aes_xcbc.cv,
-  csbcpb-cpb.aes_xcbc.out_cv_mac, AES_BLOCK_SIZE);
-   }
+
+   total = sctx-count + len;
 
/* 2 cases for total data len:
 *  1: = AES_BLOCK_SIZE: copy into state, return 0
 *  2:  AES_BLOCK_SIZE: process X blocks, copy in leftover
 */
-   if (len + sctx-count = AES_BLOCK_SIZE) {
+   if (total = AES_BLOCK_SIZE) {
memcpy(sctx-buffer + sctx-count, data, len);
sctx-count += len;
goto out;
}
 
-   /* to_process: the AES_BLOCK_SIZE data chunk to process in this
-* update */
-   to_process = (sctx-count + len)  ~(AES_BLOCK_SIZE - 1);
-   leftover = (sctx-count + len)  (AES_BLOCK_SIZE - 1);
+   in_sg = nx_ctx-in_sg;
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+   nx_ctx-ap-sglen);
 
-   /* the hardware will not accept a 0 byte operation for this algorithm
-* and the operation MUST be finalized to be correct. So if we happen
-* to get an update that falls on a block sized boundary, we must
-* save off the last block to finalize with later. */
-   if (!leftover) {
-   to_process -= AES_BLOCK_SIZE;
-   leftover = AES_BLOCK_SIZE;
-   }
+   do {
 
-   if (sctx-count) {
-   in_sg = nx_build_sg_list(nx_ctx-in_sg, sctx-buffer,
-sctx-count, nx_ctx-ap-sglen);
-   in_sg = nx_build_sg_list(in_sg, (u8 *)data,
-to_process - sctx-count,
-nx_ctx-ap-sglen);
-   nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) *
-   sizeof(struct nx_sg);
-   } else {
-   in_sg = nx_build_sg_list(nx_ctx-in_sg, (u8 *)data, to_process,
-nx_ctx-ap-sglen);
+   /* to_process: the AES_BLOCK_SIZE data chunk to process in this
+* update */
+   to_process = min_t(u64, total, nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+   NX_PAGE_SIZE * (max_sg_len - 1));
+   to_process = to_process  ~(AES_BLOCK_SIZE - 1);
+   leftover = total - to_process;
+
+   /* the hardware will not accept a 0 byte operation for this
+* algorithm and the operation MUST be finalized to be correct.
+* So if we happen to get an update that falls on a block sized
+* boundary, we must save off the last block to finalize with
+* later. */
+   if (!leftover) {
+   to_process -= AES_BLOCK_SIZE;
+   leftover = AES_BLOCK_SIZE;
+   }
+
+   if (sctx-count) {
+   in_sg = nx_build_sg_list(nx_ctx-in_sg,
+   (u8 *) sctx-buffer,
+   sctx-count,
+   max_sg_len);
+   }
+   in_sg = nx_build_sg_list(in_sg,
+   (u8 *) data,
+   to_process - sctx-count,
+   max_sg_len);
nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) *
sizeof(struct nx_sg);
-   }
 
-   NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+   /* we've hit the nx chip previously and we're updating again

[PATCH v2 00/10] Series of fixes for NX driver

[Marcelo Cerri]

This series of patches contains fixes in several algorithms implemented
by the NX driver. The patches can be separated in three different
categories:
  
 - Changes to split the data in several hyper calls to respect the
   limits of data that the co-processador can handle. This affects
   all AES modes.
 - Fixes in how the driver handle zero length messages. This affects
   XCBC and GCM.
 - Fixes for SHA-2 when chunks bigger than the block size are provided.

v2:
 - Fixed conflict.

Fionnuala Gunter (2):
  crypto: nx - fix limits to sg lists for AES-XCBC
  crypto: nx - fix limits to sg lists for AES-CCM

Marcelo Cerri (8):
  crypto: nx - add offset to nx_build_sg_lists()
  crypto: nx - fix limits to sg lists for AES-ECB
  crypto: nx - fix limits to sg lists for AES-CBC
  crypto: nx - fix limits to sg lists for AES-CTR
  crypto: nx - fix limits to sg lists for AES-GCM
  crypto: nx - fix XCBC for zero length messages
  crypto: nx - fix GCM for zero length messages
  crypto: nx - fix SHA-2 for chunks bigger than block size

 drivers/crypto/nx/nx-aes-cbc.c  |  50 ---
 drivers/crypto/nx/nx-aes-ccm.c  | 297 +---
 drivers/crypto/nx/nx-aes-ctr.c  |  50 ---
 drivers/crypto/nx/nx-aes-ecb.c  |  48 ---
 drivers/crypto/nx/nx-aes-gcm.c  | 292 ++-
 drivers/crypto/nx/nx-aes-xcbc.c | 191 +++---
 drivers/crypto/nx/nx-sha256.c   |   2 +-
 drivers/crypto/nx/nx-sha512.c   |   2 +-
 drivers/crypto/nx/nx.c  |   9 +-
 drivers/crypto/nx/nx.h  |   2 +-
 10 files changed, 683 insertions(+), 260 deletions(-)

-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/10] crypto: nx - fix limits to sg lists for AES-GCM

[Marcelo Cerri]

This patch updates the nx-aes-gcm implementation to perform several
hyper calls if needed in order to always respect the length limits for
scatter/gather lists.

Two different limits are considered:

 - ibm,max-sg-len: maximum number of bytes of each scatter/gather
   list.

 - ibm,max-sync-cop:
- The total number of bytes that a scatter/gather list can hold.
- The maximum number of elements that a scatter/gather list can have.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-gcm.c | 202 +++--
 1 file changed, 136 insertions(+), 66 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-gcm.c b/drivers/crypto/nx/nx-aes-gcm.c
index c2d6f76..9e89bdf 100644
--- a/drivers/crypto/nx/nx-aes-gcm.c
+++ b/drivers/crypto/nx/nx-aes-gcm.c
@@ -125,37 +125,101 @@ static int nx_gca(struct nx_crypto_ctx  *nx_ctx,
  struct aead_request   *req,
  u8*out)
 {
+   int rc;
struct nx_csbcpb *csbcpb_aead = nx_ctx-csbcpb_aead;
-   int rc = -EINVAL;
struct scatter_walk walk;
struct nx_sg *nx_sg = nx_ctx-in_sg;
+   unsigned int nbytes = req-assoclen;
+   unsigned int processed = 0, to_process;
+   u32 max_sg_len;
 
-   if (req-assoclen  nx_ctx-ap-databytelen)
-   goto out;
-
-   if (req-assoclen = AES_BLOCK_SIZE) {
+   if (nbytes = AES_BLOCK_SIZE) {
scatterwalk_start(walk, req-assoc);
-   scatterwalk_copychunks(out, walk, req-assoclen,
-  SCATTERWALK_FROM_SG);
+   scatterwalk_copychunks(out, walk, nbytes, SCATTERWALK_FROM_SG);
scatterwalk_done(walk, SCATTERWALK_FROM_SG, 0);
-
-   rc = 0;
-   goto out;
+   return 0;
}
 
-   nx_sg = nx_walk_and_build(nx_sg, nx_ctx-ap-sglen, req-assoc, 0,
- req-assoclen);
-   nx_ctx-op_aead.inlen = (nx_ctx-in_sg - nx_sg) * sizeof(struct nx_sg);
+   NX_CPB_FDM(csbcpb_aead) = ~NX_FDM_CONTINUATION;
 
-   rc = nx_hcall_sync(nx_ctx, nx_ctx-op_aead,
-  req-base.flags  CRYPTO_TFM_REQ_MAY_SLEEP);
-   if (rc)
-   goto out;
+   /* page_limit: number of sg entries that fit on one page */
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+  nx_ctx-ap-sglen);
 
-   atomic_inc((nx_ctx-stats-aes_ops));
-   atomic64_add(req-assoclen, (nx_ctx-stats-aes_bytes));
+   do {
+   /*
+* to_process: the data chunk to process in this update.
+* This value is bound by sg list limits.
+*/
+   to_process = min_t(u64, nbytes - processed,
+  nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+  NX_PAGE_SIZE * (max_sg_len - 1));
+
+   if ((to_process + processed)  nbytes)
+   NX_CPB_FDM(csbcpb_aead) |= NX_FDM_INTERMEDIATE;
+   else
+   NX_CPB_FDM(csbcpb_aead) = ~NX_FDM_INTERMEDIATE;
+
+   nx_sg = nx_walk_and_build(nx_ctx-in_sg, nx_ctx-ap-sglen,
+ req-assoc, processed, to_process);
+   nx_ctx-op_aead.inlen = (nx_ctx-in_sg - nx_sg)
+   * sizeof(struct nx_sg);
+
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op_aead,
+   req-base.flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   return rc;
+
+   memcpy(csbcpb_aead-cpb.aes_gca.in_pat,
+   csbcpb_aead-cpb.aes_gca.out_pat,
+   AES_BLOCK_SIZE);
+   NX_CPB_FDM(csbcpb_aead) |= NX_FDM_CONTINUATION;
+
+   atomic_inc((nx_ctx-stats-aes_ops));
+   atomic64_add(req-assoclen, (nx_ctx-stats-aes_bytes));
+
+   processed += to_process;
+   } while (processed  nbytes);
 
memcpy(out, csbcpb_aead-cpb.aes_gca.out_pat, AES_BLOCK_SIZE);
+
+   return rc;
+}
+
+static int gcm_empty(struct aead_request *req, struct blkcipher_desc *desc,
+int enc)
+{
+   int rc;
+   struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(req-base.tfm);
+   struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
+
+   /* For scenarios where the input message is zero length, AES CTR mode
+* may be used. Set the source data to be a single block (16B) of all
+* zeros, and set the input IV value to be the same as the GMAC IV
+* value. - nx_wb 4.8.1.3 */
+   char src[AES_BLOCK_SIZE] = {};
+   struct scatterlist sg;
+
+   desc-tfm = crypto_alloc_blkcipher(ctr(aes), 0, 0);
+   if (IS_ERR(desc-tfm)) {
+   rc = -ENOMEM

[PATCH 09/10] crypto: nx - fix GCM for zero length messages

[Marcelo Cerri]

The NX CGM implementation doesn't support zero length messages and the
current implementation has two flaws:

 - When the input data length is zero, it ignores the associated data.
 - Even when both lengths are zero, it uses the Crypto API to encrypt a
   zeroed block using ctr(aes) and because of this it allocates a new
   transformation and sets the key for this new tfm. Both operations are
   intended to be used only in user context, while the cryptographic
   operations can be called in both user and softirq contexts.

This patch replaces the nested Crypto API use and adds two special
cases:

 - When input data and associated data lengths are zero: it uses NX ECB
   mode to emulate the encryption of a zeroed block using ctr(aes).
 - When input data is zero and associated data is available: it uses NX
   GMAC mode to calculate the associated data MAC.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-gcm.c | 132 ++---
 1 file changed, 112 insertions(+), 20 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-gcm.c b/drivers/crypto/nx/nx-aes-gcm.c
index 9e89bdf..025d9a8 100644
--- a/drivers/crypto/nx/nx-aes-gcm.c
+++ b/drivers/crypto/nx/nx-aes-gcm.c
@@ -187,40 +187,125 @@ static int nx_gca(struct nx_crypto_ctx  *nx_ctx,
return rc;
 }
 
+static int gmac(struct aead_request *req, struct blkcipher_desc *desc)
+{
+   int rc;
+   struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(req-base.tfm);
+   struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
+   struct nx_sg *nx_sg;
+   unsigned int nbytes = req-assoclen;
+   unsigned int processed = 0, to_process;
+   u32 max_sg_len;
+
+   /* Set GMAC mode */
+   csbcpb-cpb.hdr.mode = NX_MODE_AES_GMAC;
+
+   NX_CPB_FDM(csbcpb) = ~NX_FDM_CONTINUATION;
+
+   /* page_limit: number of sg entries that fit on one page */
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+  nx_ctx-ap-sglen);
+
+   /* Copy IV */
+   memcpy(csbcpb-cpb.aes_gcm.iv_or_cnt, desc-info, AES_BLOCK_SIZE);
+
+   do {
+   /*
+* to_process: the data chunk to process in this update.
+* This value is bound by sg list limits.
+*/
+   to_process = min_t(u64, nbytes - processed,
+  nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+  NX_PAGE_SIZE * (max_sg_len - 1));
+
+   if ((to_process + processed)  nbytes)
+   NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+   else
+   NX_CPB_FDM(csbcpb) = ~NX_FDM_INTERMEDIATE;
+
+   nx_sg = nx_walk_and_build(nx_ctx-in_sg, nx_ctx-ap-sglen,
+ req-assoc, processed, to_process);
+   nx_ctx-op.inlen = (nx_ctx-in_sg - nx_sg)
+   * sizeof(struct nx_sg);
+
+   csbcpb-cpb.aes_gcm.bit_length_data = 0;
+   csbcpb-cpb.aes_gcm.bit_length_aad = 8 * nbytes;
+
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
+   req-base.flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   goto out;
+
+   memcpy(csbcpb-cpb.aes_gcm.in_pat_or_aad,
+   csbcpb-cpb.aes_gcm.out_pat_or_mac, AES_BLOCK_SIZE);
+   memcpy(csbcpb-cpb.aes_gcm.in_s0,
+   csbcpb-cpb.aes_gcm.out_s0, AES_BLOCK_SIZE);
+
+   NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
+
+   atomic_inc((nx_ctx-stats-aes_ops));
+   atomic64_add(req-assoclen, (nx_ctx-stats-aes_bytes));
+
+   processed += to_process;
+   } while (processed  nbytes);
+
+out:
+   /* Restore GCM mode */
+   csbcpb-cpb.hdr.mode = NX_MODE_AES_GCM;
+   return rc;
+}
+
 static int gcm_empty(struct aead_request *req, struct blkcipher_desc *desc,
 int enc)
 {
int rc;
struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(req-base.tfm);
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
+   char out[AES_BLOCK_SIZE];
+   struct nx_sg *in_sg, *out_sg;
 
/* For scenarios where the input message is zero length, AES CTR mode
 * may be used. Set the source data to be a single block (16B) of all
 * zeros, and set the input IV value to be the same as the GMAC IV
 * value. - nx_wb 4.8.1.3 */
-   char src[AES_BLOCK_SIZE] = {};
-   struct scatterlist sg;
 
-   desc-tfm = crypto_alloc_blkcipher(ctr(aes), 0, 0);
-   if (IS_ERR(desc-tfm)) {
-   rc = -ENOMEM;
-   goto out;
-   }
-
-   crypto_blkcipher_setkey(desc-tfm, csbcpb-cpb.aes_gcm.key,
-   NX_CPB_KEY_SIZE(csbcpb) == NX_KS_AES_128 ? 16 :
-   NX_CPB_KEY_SIZE

[PATCH 08/10] crypto: nx - fix XCBC for zero length messages

[Marcelo Cerri]

The NX XCBC implementation doesn't support zero length messages and
because of that NX is currently returning a hard-coded hash for zero
length messages. However this approach is incorrect since the hash value
also depends on which key is used.

This patch removes the hard-coded hash and replace it with an
implementation based on the RFC 3566 using ECB.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-xcbc.c | 84 +
 1 file changed, 77 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-xcbc.c b/drivers/crypto/nx/nx-aes-xcbc.c
index 1a5d9e3..03c4bf5 100644
--- a/drivers/crypto/nx/nx-aes-xcbc.c
+++ b/drivers/crypto/nx/nx-aes-xcbc.c
@@ -56,6 +56,77 @@ static int nx_xcbc_set_key(struct crypto_shash *desc,
return 0;
 }
 
+/*
+ * Based on RFC 3566, for a zero-length message:
+ *
+ * n = 1
+ * K1 = E(K, 0x01010101010101010101010101010101)
+ * K3 = E(K, 0x03030303030303030303030303030303)
+ * E[0] = 0x
+ * M[1] = 0x8000 (0 length message with padding)
+ * E[1] = (K1, M[1] ^ E[0] ^ K3)
+ * Tag = M[1]
+ */
+static int nx_xcbc_empty(struct shash_desc *desc, u8 *out)
+{
+   struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(desc-tfm-base);
+   struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
+   struct nx_sg *in_sg, *out_sg;
+   u8 keys[2][AES_BLOCK_SIZE];
+   u8 key[32];
+   int rc = 0;
+
+   /* Change to ECB mode */
+   csbcpb-cpb.hdr.mode = NX_MODE_AES_ECB;
+   memcpy(key, csbcpb-cpb.aes_xcbc.key, AES_BLOCK_SIZE);
+   memcpy(csbcpb-cpb.aes_ecb.key, key, AES_BLOCK_SIZE);
+   NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
+
+   /* K1 and K3 base patterns */
+   memset(keys[0], 0x01, sizeof(keys[0]));
+   memset(keys[1], 0x03, sizeof(keys[1]));
+
+   /* Generate K1 and K3 encrypting the patterns */
+   in_sg = nx_build_sg_list(nx_ctx-in_sg, (u8 *) keys, sizeof(keys),
+nx_ctx-ap-sglen);
+   out_sg = nx_build_sg_list(nx_ctx-out_sg, (u8 *) keys, sizeof(keys),
+ nx_ctx-ap-sglen);
+   nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) * sizeof(struct nx_sg);
+   nx_ctx-op.outlen = (nx_ctx-out_sg - out_sg) * sizeof(struct nx_sg);
+
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
+  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   goto out;
+   atomic_inc((nx_ctx-stats-aes_ops));
+
+   /* XOr K3 with the padding for a 0 length message */
+   keys[1][0] ^= 0x80;
+
+   /* Encrypt the final result */
+   memcpy(csbcpb-cpb.aes_ecb.key, keys[0], AES_BLOCK_SIZE);
+   in_sg = nx_build_sg_list(nx_ctx-in_sg, (u8 *) keys[1], sizeof(keys[1]),
+nx_ctx-ap-sglen);
+   out_sg = nx_build_sg_list(nx_ctx-out_sg, out, AES_BLOCK_SIZE,
+ nx_ctx-ap-sglen);
+   nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) * sizeof(struct nx_sg);
+   nx_ctx-op.outlen = (nx_ctx-out_sg - out_sg) * sizeof(struct nx_sg);
+
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
+  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   goto out;
+   atomic_inc((nx_ctx-stats-aes_ops));
+
+out:
+   /* Restore XCBC mode */
+   csbcpb-cpb.hdr.mode = NX_MODE_AES_XCBC_MAC;
+   memcpy(csbcpb-cpb.aes_xcbc.key, key, AES_BLOCK_SIZE);
+   NX_CPB_FDM(csbcpb) = ~NX_FDM_ENDE_ENCRYPT;
+
+   return rc;
+}
+
 static int nx_xcbc_init(struct shash_desc *desc)
 {
struct xcbc_state *sctx = shash_desc_ctx(desc);
@@ -201,13 +272,12 @@ static int nx_xcbc_final(struct shash_desc *desc, u8 *out)
memcpy(csbcpb-cpb.aes_xcbc.cv,
   csbcpb-cpb.aes_xcbc.out_cv_mac, AES_BLOCK_SIZE);
} else if (sctx-count == 0) {
-   /* we've never seen an update, so this is a 0 byte op. The
-* hardware cannot handle a 0 byte op, so just copy out the
-* known 0 byte result. This is cheaper than allocating a
-* software context to do a 0 byte op */
-   u8 data[] = { 0x75, 0xf0, 0x25, 0x1d, 0x52, 0x8a, 0xc0, 0x1c,
- 0x45, 0x73, 0xdf, 0xd5, 0x84, 0xd7, 0x9f, 0x29 };
-   memcpy(out, data, sizeof(data));
+   /*
+* we've never seen an update, so this is a 0 byte op. The
+* hardware cannot handle a 0 byte op, so just ECB to
+* generate the hash.
+*/
+   rc = nx_xcbc_empty(desc, out);
goto out;
}
 
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/10] crypto: nx - fix SHA-2 for chunks bigger than block size

[Marcelo Cerri]

Each call to the co-processor, with exception of the last call, needs to
send data that is multiple of block size. As consequence, any remaining
data is kept in the internal NX context.

This patch fixes a bug in the driver that causes it to save incorrect
data into the context when data is bigger than the block size.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-sha256.c | 2 +-
 drivers/crypto/nx/nx-sha512.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/nx/nx-sha256.c b/drivers/crypto/nx/nx-sha256.c
index 6547a71..da0b24a 100644
--- a/drivers/crypto/nx/nx-sha256.c
+++ b/drivers/crypto/nx/nx-sha256.c
@@ -129,7 +129,7 @@ static int nx_sha256_update(struct shash_desc *desc, const 
u8 *data,
NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
 
total -= to_process;
-   data += to_process;
+   data += to_process - sctx-count;
sctx-count = 0;
in_sg = nx_ctx-in_sg;
} while (leftover = SHA256_BLOCK_SIZE);
diff --git a/drivers/crypto/nx/nx-sha512.c b/drivers/crypto/nx/nx-sha512.c
index 236e6af..4ae5b0f 100644
--- a/drivers/crypto/nx/nx-sha512.c
+++ b/drivers/crypto/nx/nx-sha512.c
@@ -131,7 +131,7 @@ static int nx_sha512_update(struct shash_desc *desc, const 
u8 *data,
NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
 
total -= to_process;
-   data += to_process;
+   data += to_process - sctx-count[0];
sctx-count[0] = 0;
in_sg = nx_ctx-in_sg;
} while (leftover = SHA512_BLOCK_SIZE);
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/10] crypto: nx - fix limits to sg lists for AES-XCBC

[Marcelo Cerri]

From: Fionnuala Gunter f...@linux.vnet.ibm.com

This patch updates the NX driver to perform several hyper calls when necessary
so that the length limits of scatter/gather lists are respected.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Reviewed-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
Signed-off-by: Fionnuala Gunter f...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-xcbc.c | 107 +++-
 1 file changed, 63 insertions(+), 44 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-xcbc.c b/drivers/crypto/nx/nx-aes-xcbc.c
index 658da0f..1a5d9e3 100644
--- a/drivers/crypto/nx/nx-aes-xcbc.c
+++ b/drivers/crypto/nx/nx-aes-xcbc.c
@@ -88,78 +88,97 @@ static int nx_xcbc_update(struct shash_desc *desc,
struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(desc-tfm-base);
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
struct nx_sg *in_sg;
-   u32 to_process, leftover;
+   u32 to_process, leftover, total;
+   u32 max_sg_len;
unsigned long irq_flags;
int rc = 0;
 
spin_lock_irqsave(nx_ctx-lock, irq_flags);
 
-   if (NX_CPB_FDM(csbcpb)  NX_FDM_CONTINUATION) {
-   /* we've hit the nx chip previously and we're updating again,
-* so copy over the partial digest */
-   memcpy(csbcpb-cpb.aes_xcbc.cv,
-  csbcpb-cpb.aes_xcbc.out_cv_mac, AES_BLOCK_SIZE);
-   }
+
+   total = sctx-count + len;
 
/* 2 cases for total data len:
 *  1: = AES_BLOCK_SIZE: copy into state, return 0
 *  2:  AES_BLOCK_SIZE: process X blocks, copy in leftover
 */
-   if (len + sctx-count = AES_BLOCK_SIZE) {
+   if (total = AES_BLOCK_SIZE) {
memcpy(sctx-buffer + sctx-count, data, len);
sctx-count += len;
goto out;
}
 
-   /* to_process: the AES_BLOCK_SIZE data chunk to process in this
-* update */
-   to_process = (sctx-count + len)  ~(AES_BLOCK_SIZE - 1);
-   leftover = (sctx-count + len)  (AES_BLOCK_SIZE - 1);
+   in_sg = nx_ctx-in_sg;
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+   nx_ctx-ap-sglen);
 
-   /* the hardware will not accept a 0 byte operation for this algorithm
-* and the operation MUST be finalized to be correct. So if we happen
-* to get an update that falls on a block sized boundary, we must
-* save off the last block to finalize with later. */
-   if (!leftover) {
-   to_process -= AES_BLOCK_SIZE;
-   leftover = AES_BLOCK_SIZE;
-   }
+   do {
 
-   if (sctx-count) {
-   in_sg = nx_build_sg_list(nx_ctx-in_sg, sctx-buffer,
-sctx-count, nx_ctx-ap-sglen);
-   in_sg = nx_build_sg_list(in_sg, (u8 *)data,
-to_process - sctx-count,
-nx_ctx-ap-sglen);
-   nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) *
-   sizeof(struct nx_sg);
-   } else {
-   in_sg = nx_build_sg_list(nx_ctx-in_sg, (u8 *)data, to_process,
-nx_ctx-ap-sglen);
+   /* to_process: the AES_BLOCK_SIZE data chunk to process in this
+* update */
+   to_process = min_t(u64, total, nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+   NX_PAGE_SIZE * (max_sg_len - 1));
+   to_process = to_process  ~(AES_BLOCK_SIZE - 1);
+   leftover = total - to_process;
+
+   /* the hardware will not accept a 0 byte operation for this
+* algorithm and the operation MUST be finalized to be correct.
+* So if we happen to get an update that falls on a block sized
+* boundary, we must save off the last block to finalize with
+* later. */
+   if (!leftover) {
+   to_process -= AES_BLOCK_SIZE;
+   leftover = AES_BLOCK_SIZE;
+   }
+
+   if (sctx-count) {
+   in_sg = nx_build_sg_list(nx_ctx-in_sg,
+   (u8 *) sctx-buffer,
+   sctx-count,
+   max_sg_len);
+   }
+   in_sg = nx_build_sg_list(in_sg,
+   (u8 *) data,
+   to_process - sctx-count,
+   max_sg_len);
nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) *
sizeof(struct nx_sg);
-   }
 
-   NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+   /* we've hit the nx chip previously and we're updating again

[PATCH 04/10] crypto: nx - fix limits to sg lists for AES-CTR

[Marcelo Cerri]

This patch updates the nx-aes-ctr implementation to perform several
hyper calls if needed in order to always respect the length limits for
scatter/gather lists.

Two different limits are considered:

 - ibm,max-sg-len: maximum number of bytes of each scatter/gather
   list.

 - ibm,max-sync-cop:
- The total number of bytes that a scatter/gather list can hold.
- The maximum number of elements that a scatter/gather list can have.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-ctr.c | 50 ++
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-ctr.c b/drivers/crypto/nx/nx-aes-ctr.c
index 80dee8d..a37d009 100644
--- a/drivers/crypto/nx/nx-aes-ctr.c
+++ b/drivers/crypto/nx/nx-aes-ctr.c
@@ -89,33 +89,45 @@ static int ctr_aes_nx_crypt(struct blkcipher_desc *desc,
struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc-tfm);
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
unsigned long irq_flags;
+   unsigned int processed = 0, to_process;
+   u32 max_sg_len;
int rc;
 
spin_lock_irqsave(nx_ctx-lock, irq_flags);
 
-   if (nbytes  nx_ctx-ap-databytelen) {
-   rc = -EINVAL;
-   goto out;
-   }
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+  nx_ctx-ap-sglen);
 
-   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0,
-  csbcpb-cpb.aes_ctr.iv);
-   if (rc)
-   goto out;
+   do {
+   to_process = min_t(u64, nbytes - processed,
+  nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+  NX_PAGE_SIZE * (max_sg_len - 1));
+   to_process = to_process  ~(AES_BLOCK_SIZE - 1);
 
-   if (!nx_ctx-op.inlen || !nx_ctx-op.outlen) {
-   rc = -EINVAL;
-   goto out;
-   }
+   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, to_process,
+  processed, csbcpb-cpb.aes_ctr.iv);
+   if (rc)
+   goto out;
 
-   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
-  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
-   if (rc)
-   goto out;
+   if (!nx_ctx-op.inlen || !nx_ctx-op.outlen) {
+   rc = -EINVAL;
+   goto out;
+   }
 
-   atomic_inc((nx_ctx-stats-aes_ops));
-   atomic64_add(csbcpb-csb.processed_byte_count,
-(nx_ctx-stats-aes_bytes));
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
+  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   goto out;
+
+   memcpy(desc-info, csbcpb-cpb.aes_cbc.cv, AES_BLOCK_SIZE);
+
+   atomic_inc((nx_ctx-stats-aes_ops));
+   atomic64_add(csbcpb-csb.processed_byte_count,
+(nx_ctx-stats-aes_bytes));
+
+   processed += to_process;
+   } while (processed  nbytes);
 out:
spin_unlock_irqrestore(nx_ctx-lock, irq_flags);
return rc;
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 07/10] crypto: nx - fix limits to sg lists for AES-CCM

[Marcelo Cerri]

From: Fionnuala Gunter f...@linux.vnet.ibm.com

This patch updates the NX driver to perform several hyper calls when necessary
so that the length limits of scatter/gather lists are respected.

Reviewed-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Fionnuala Gunter f...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-ccm.c | 297 +
 1 file changed, 215 insertions(+), 82 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-ccm.c b/drivers/crypto/nx/nx-aes-ccm.c
index 666a35b..5ecd4c2 100644
--- a/drivers/crypto/nx/nx-aes-ccm.c
+++ b/drivers/crypto/nx/nx-aes-ccm.c
@@ -179,13 +179,26 @@ static int generate_pat(u8   *iv,
struct nx_sg *nx_insg = nx_ctx-in_sg;
struct nx_sg *nx_outsg = nx_ctx-out_sg;
unsigned int iauth_len = 0;
-   struct vio_pfo_op *op = NULL;
u8 tmp[16], *b1 = NULL, *b0 = NULL, *result = NULL;
int rc;
 
/* zero the ctr value */
memset(iv + 15 - iv[0], 0, iv[0] + 1);
 
+   /* page 78 of nx_wb.pdf has,
+* Note: RFC3610 allows the AAD data to be up to 2^64 -1 bytes
+* in length. If a full message is used, the AES CCA implementation
+* restricts the maximum AAD length to 2^32 -1 bytes.
+* If partial messages are used, the implementation supports
+* 2^64 -1 bytes maximum AAD length.
+*
+* However, in the cryptoapi's aead_request structure,
+* assoclen is an unsigned int, thus it cannot hold a length
+* value greater than 2^32 - 1.
+* Thus the AAD is further constrained by this and is never
+* greater than 2^32.
+*/
+
if (!req-assoclen) {
b0 = nx_ctx-csbcpb-cpb.aes_ccm.in_pat_or_b0;
} else if (req-assoclen = 14) {
@@ -195,7 +208,46 @@ static int generate_pat(u8   *iv,
b0 = nx_ctx-csbcpb-cpb.aes_ccm.in_pat_or_b0;
b1 = nx_ctx-priv.ccm.iauth_tag;
iauth_len = req-assoclen;
+   } else if (req-assoclen = 65280) {
+   /* if associated data is less than (2^16 - 2^8), we construct
+* B1 differently and feed in the associated data to a CCA
+* operation */
+   b0 = nx_ctx-csbcpb_aead-cpb.aes_cca.b0;
+   b1 = nx_ctx-csbcpb_aead-cpb.aes_cca.b1;
+   iauth_len = 14;
+   } else {
+   b0 = nx_ctx-csbcpb_aead-cpb.aes_cca.b0;
+   b1 = nx_ctx-csbcpb_aead-cpb.aes_cca.b1;
+   iauth_len = 10;
+   }
 
+   /* generate B0 */
+   rc = generate_b0(iv, req-assoclen, authsize, nbytes, b0);
+   if (rc)
+   return rc;
+
+   /* generate B1:
+* add control info for associated data
+* RFC 3610 and NIST Special Publication 800-38C
+*/
+   if (b1) {
+   memset(b1, 0, 16);
+   if (req-assoclen = 65280) {
+   *(u16 *)b1 = (u16)req-assoclen;
+   scatterwalk_map_and_copy(b1 + 2, req-assoc, 0,
+iauth_len, SCATTERWALK_FROM_SG);
+   } else {
+   *(u16 *)b1 = (u16)(0xfffe);
+   *(u32 *)b1[2] = (u32)req-assoclen;
+   scatterwalk_map_and_copy(b1 + 6, req-assoc, 0,
+iauth_len, SCATTERWALK_FROM_SG);
+   }
+   }
+
+   /* now copy any remaining AAD to scatterlist and call nx... */
+   if (!req-assoclen) {
+   return rc;
+   } else if (req-assoclen = 14) {
nx_insg = nx_build_sg_list(nx_insg, b1, 16, nx_ctx-ap-sglen);
nx_outsg = nx_build_sg_list(nx_outsg, tmp, 16,
nx_ctx-ap-sglen);
@@ -210,56 +262,74 @@ static int generate_pat(u8   *iv,
NX_CPB_FDM(nx_ctx-csbcpb) |= NX_FDM_ENDE_ENCRYPT;
NX_CPB_FDM(nx_ctx-csbcpb) |= NX_FDM_INTERMEDIATE;
 
-   op = nx_ctx-op;
result = nx_ctx-csbcpb-cpb.aes_ccm.out_pat_or_mac;
-   } else if (req-assoclen = 65280) {
-   /* if associated data is less than (2^16 - 2^8), we construct
-* B1 differently and feed in the associated data to a CCA
-* operation */
-   b0 = nx_ctx-csbcpb_aead-cpb.aes_cca.b0;
-   b1 = nx_ctx-csbcpb_aead-cpb.aes_cca.b1;
-   iauth_len = 14;
-
-   /* remaining assoc data must have scatterlist built for it */
-   nx_insg = nx_walk_and_build(nx_insg, nx_ctx-ap-sglen,
-   req-assoc, iauth_len,
-   req-assoclen - iauth_len);
-   nx_ctx-op_aead.inlen = (nx_ctx-in_sg - nx_insg) *
+
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op

[PATCH 03/10] crypto: nx - fix limits to sg lists for AES-CBC

[Marcelo Cerri]

This patch updates the nx-aes-cbc implementation to perform several
hyper calls if needed in order to always respect the length limits for
scatter/gather lists.

Two different limits are considered:

 - ibm,max-sg-len: maximum number of bytes of each scatter/gather
   list.

 - ibm,max-sync-cop:
- The total number of bytes that a scatter/gather list can hold.
- The maximum number of elements that a scatter/gather list can have.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-cbc.c | 50 +-
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c
index f334a60..fa37df1 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -71,40 +71,50 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc *desc,
struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc-tfm);
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
unsigned long irq_flags;
+   unsigned int processed = 0, to_process;
+   u32 max_sg_len;
int rc;
 
spin_lock_irqsave(nx_ctx-lock, irq_flags);
 
-   if (nbytes  nx_ctx-ap-databytelen) {
-   rc = -EINVAL;
-   goto out;
-   }
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+  nx_ctx-ap-sglen);
 
if (enc)
NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
else
NX_CPB_FDM(csbcpb) = ~NX_FDM_ENDE_ENCRYPT;
 
-   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0,
-  csbcpb-cpb.aes_cbc.iv);
-   if (rc)
-   goto out;
+   do {
+   to_process = min_t(u64, nbytes - processed,
+  nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+  NX_PAGE_SIZE * (max_sg_len - 1));
+   to_process = to_process  ~(AES_BLOCK_SIZE - 1);
 
-   if (!nx_ctx-op.inlen || !nx_ctx-op.outlen) {
-   rc = -EINVAL;
-   goto out;
-   }
+   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, to_process,
+  processed, csbcpb-cpb.aes_cbc.iv);
+   if (rc)
+   goto out;
 
-   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
-  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
-   if (rc)
-   goto out;
+   if (!nx_ctx-op.inlen || !nx_ctx-op.outlen) {
+   rc = -EINVAL;
+   goto out;
+   }
 
-   memcpy(desc-info, csbcpb-cpb.aes_cbc.cv, AES_BLOCK_SIZE);
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
+  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   goto out;
 
-   atomic_inc((nx_ctx-stats-aes_ops));
-   atomic64_add(csbcpb-csb.processed_byte_count,
-(nx_ctx-stats-aes_bytes));
+   memcpy(desc-info, csbcpb-cpb.aes_cbc.cv, AES_BLOCK_SIZE);
+
+   atomic_inc((nx_ctx-stats-aes_ops));
+   atomic64_add(csbcpb-csb.processed_byte_count,
+(nx_ctx-stats-aes_bytes));
+
+   processed += to_process;
+   } while (processed  nbytes);
 out:
spin_unlock_irqrestore(nx_ctx-lock, irq_flags);
return rc;
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/10] crypto: nx - fix limits to sg lists for AES-ECB

[Marcelo Cerri]

This patch updates the nx-aes-ecb implementation to perform several
hyper calls if needed in order to always respect the length limits for
scatter/gather lists.

Two different limits are considered:

 - ibm,max-sg-len: maximum number of bytes of each scatter/gather
   list.

 - ibm,max-sync-cop:
- The total number of bytes that a scatter/gather list can hold.
- The maximum number of elements that a scatter/gather list can have.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-ecb.c | 48 ++
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-ecb.c b/drivers/crypto/nx/nx-aes-ecb.c
index fe0d803..85a8d23 100644
--- a/drivers/crypto/nx/nx-aes-ecb.c
+++ b/drivers/crypto/nx/nx-aes-ecb.c
@@ -71,37 +71,49 @@ static int ecb_aes_nx_crypt(struct blkcipher_desc *desc,
struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc-tfm);
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
unsigned long irq_flags;
+   unsigned int processed = 0, to_process;
+   u32 max_sg_len;
int rc;
 
spin_lock_irqsave(nx_ctx-lock, irq_flags);
 
-   if (nbytes  nx_ctx-ap-databytelen) {
-   rc = -EINVAL;
-   goto out;
-   }
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+  nx_ctx-ap-sglen);
 
if (enc)
NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
else
NX_CPB_FDM(csbcpb) = ~NX_FDM_ENDE_ENCRYPT;
 
-   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0, NULL);
-   if (rc)
-   goto out;
+   do {
+   to_process = min_t(u64, nbytes - processed,
+  nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+  NX_PAGE_SIZE * (max_sg_len - 1));
+   to_process = to_process  ~(AES_BLOCK_SIZE - 1);
 
-   if (!nx_ctx-op.inlen || !nx_ctx-op.outlen) {
-   rc = -EINVAL;
-   goto out;
-   }
+   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, to_process,
+   processed, NULL);
+   if (rc)
+   goto out;
 
-   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
-  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
-   if (rc)
-   goto out;
+   if (!nx_ctx-op.inlen || !nx_ctx-op.outlen) {
+   rc = -EINVAL;
+   goto out;
+   }
+
+   rc = nx_hcall_sync(nx_ctx, nx_ctx-op,
+  desc-flags  CRYPTO_TFM_REQ_MAY_SLEEP);
+   if (rc)
+   goto out;
+
+   atomic_inc((nx_ctx-stats-aes_ops));
+   atomic64_add(csbcpb-csb.processed_byte_count,
+(nx_ctx-stats-aes_bytes));
+
+   processed += to_process;
+   } while (processed  nbytes);
 
-   atomic_inc((nx_ctx-stats-aes_ops));
-   atomic64_add(csbcpb-csb.processed_byte_count,
-(nx_ctx-stats-aes_bytes));
 out:
spin_unlock_irqrestore(nx_ctx-lock, irq_flags);
return rc;
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 00/10] Series of fixes for NX driver

[Marcelo Cerri]

This series of patches contains fixes in several algorithms implemented
by the NX driver. The patches can be separated in three different
categories:

 - Changes to split the data in several hyper calls to respect the
   limits of data that the co-processador can handle. This affects
   all AES modes.
 - Fixes in how the driver handle zero length messages. This affects
   XCBC and GCM.
 - Fixes for SHA-2 when chunks bigger than the block size are provided.

Fionnuala Gunter (2):
  crypto: nx - fix limits to sg lists for AES-XCBC
  crypto: nx - fix limits to sg lists for AES-CCM

Marcelo Cerri (8):
  crypto: nx - add offset to nx_build_sg_lists()
  crypto: nx - fix limits to sg lists for AES-ECB
  crypto: nx - fix limits to sg lists for AES-CBC
  crypto: nx - fix limits to sg lists for AES-CTR
  crypto: nx - fix limits to sg lists for AES-GCM
  crypto: nx - fix XCBC for zero length messages
  crypto: nx - fix GCM for zero length messages
  crypto: nx - fix SHA-2 for chunks bigger than block size

 drivers/crypto/nx/nx-aes-cbc.c  |  50 ---
 drivers/crypto/nx/nx-aes-ccm.c  | 297 +---
 drivers/crypto/nx/nx-aes-ctr.c  |  50 ---
 drivers/crypto/nx/nx-aes-ecb.c  |  48 ---
 drivers/crypto/nx/nx-aes-gcm.c  | 292 ++-
 drivers/crypto/nx/nx-aes-xcbc.c | 191 +++---
 drivers/crypto/nx/nx-sha256.c   |   2 +-
 drivers/crypto/nx/nx-sha512.c   |   2 +-
 drivers/crypto/nx/nx.c  |   9 +-
 drivers/crypto/nx/nx.h  |   2 +-
 10 files changed, 683 insertions(+), 260 deletions(-)

-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/10] crypto: nx - add offset to nx_build_sg_lists()

[Marcelo Cerri]

This patch includes one more parameter to nx_build_sg_lists() to skip
the given number of bytes from beginning of each sg list.

This is needed in order to implement the fixes for the AES modes to make
them able to process larger chunks of data.

Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-cbc.c | 2 +-
 drivers/crypto/nx/nx-aes-ccm.c | 4 ++--
 drivers/crypto/nx/nx-aes-ctr.c | 2 +-
 drivers/crypto/nx/nx-aes-ecb.c | 2 +-
 drivers/crypto/nx/nx-aes-gcm.c | 2 +-
 drivers/crypto/nx/nx.c | 9 +++--
 drivers/crypto/nx/nx.h | 2 +-
 7 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c
index 9310982..f334a60 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -85,7 +85,7 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc *desc,
else
NX_CPB_FDM(csbcpb) = ~NX_FDM_ENDE_ENCRYPT;
 
-   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes,
+   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0,
   csbcpb-cpb.aes_cbc.iv);
if (rc)
goto out;
diff --git a/drivers/crypto/nx/nx-aes-ccm.c b/drivers/crypto/nx/nx-aes-ccm.c
index 39d4224..666a35b 100644
--- a/drivers/crypto/nx/nx-aes-ccm.c
+++ b/drivers/crypto/nx/nx-aes-ccm.c
@@ -293,7 +293,7 @@ static int ccm_nx_decrypt(struct aead_request   *req,
if (rc)
goto out;
 
-   rc = nx_build_sg_lists(nx_ctx, desc, req-dst, req-src, nbytes,
+   rc = nx_build_sg_lists(nx_ctx, desc, req-dst, req-src, nbytes, 0,
   csbcpb-cpb.aes_ccm.iv_or_ctr);
if (rc)
goto out;
@@ -339,7 +339,7 @@ static int ccm_nx_encrypt(struct aead_request   *req,
if (rc)
goto out;
 
-   rc = nx_build_sg_lists(nx_ctx, desc, req-dst, req-src, nbytes,
+   rc = nx_build_sg_lists(nx_ctx, desc, req-dst, req-src, nbytes, 0,
   csbcpb-cpb.aes_ccm.iv_or_ctr);
if (rc)
goto out;
diff --git a/drivers/crypto/nx/nx-aes-ctr.c b/drivers/crypto/nx/nx-aes-ctr.c
index 762611b..80dee8d 100644
--- a/drivers/crypto/nx/nx-aes-ctr.c
+++ b/drivers/crypto/nx/nx-aes-ctr.c
@@ -98,7 +98,7 @@ static int ctr_aes_nx_crypt(struct blkcipher_desc *desc,
goto out;
}
 
-   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes,
+   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0,
   csbcpb-cpb.aes_ctr.iv);
if (rc)
goto out;
diff --git a/drivers/crypto/nx/nx-aes-ecb.c b/drivers/crypto/nx/nx-aes-ecb.c
index 77dbe08..fe0d803 100644
--- a/drivers/crypto/nx/nx-aes-ecb.c
+++ b/drivers/crypto/nx/nx-aes-ecb.c
@@ -85,7 +85,7 @@ static int ecb_aes_nx_crypt(struct blkcipher_desc *desc,
else
NX_CPB_FDM(csbcpb) = ~NX_FDM_ENDE_ENCRYPT;
 
-   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, NULL);
+   rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0, NULL);
if (rc)
goto out;
 
diff --git a/drivers/crypto/nx/nx-aes-gcm.c b/drivers/crypto/nx/nx-aes-gcm.c
index 74feee1..c2d6f76 100644
--- a/drivers/crypto/nx/nx-aes-gcm.c
+++ b/drivers/crypto/nx/nx-aes-gcm.c
@@ -226,7 +226,7 @@ static int gcm_aes_nx_crypt(struct aead_request *req, int 
enc)
 
csbcpb-cpb.aes_gcm.bit_length_data = nbytes * 8;
 
-   rc = nx_build_sg_lists(nx_ctx, desc, req-dst, req-src, nbytes,
+   rc = nx_build_sg_lists(nx_ctx, desc, req-dst, req-src, nbytes, 0,
   csbcpb-cpb.aes_gcm.iv_or_cnt);
if (rc)
goto out;
diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c
index bdf4990..5533fe3 100644
--- a/drivers/crypto/nx/nx.c
+++ b/drivers/crypto/nx/nx.c
@@ -211,6 +211,8 @@ struct nx_sg *nx_walk_and_build(struct nx_sg   *nx_dst,
  * @dst: destination scatterlist
  * @src: source scatterlist
  * @nbytes: length of data described in the scatterlists
+ * @offset: number of bytes to fast-forward past at the beginning of
+ *  scatterlists.
  * @iv: destination for the iv data, if the algorithm requires it
  *
  * This is common code shared by all the AES algorithms. It uses the block
@@ -222,6 +224,7 @@ int nx_build_sg_lists(struct nx_crypto_ctx  *nx_ctx,
  struct scatterlist*dst,
  struct scatterlist*src,
  unsigned int   nbytes,
+ unsigned int   offset,
  u8*iv)
 {
struct nx_sg *nx_insg = nx_ctx-in_sg;
@@ -230,8 +233,10 @@ int nx_build_sg_lists(struct nx_crypto_ctx  *nx_ctx,
if (iv)
memcpy(iv, desc-info, AES_BLOCK_SIZE);
 
-   nx_insg = nx_walk_and_build(nx_insg, nx_ctx-ap-sglen, src, 0, nbytes);
-   nx_outsg

Re: Questions about the Crypto API

[Marcelo Cerri]

On Sat, Aug 10, 2013 at 11:15:41AM +1000, Herbert Xu wrote:
 On Fri, Aug 09, 2013 at 01:09:12PM +, Hsieh, Che-Min wrote:
  Marcelo/Herbert:
  
  I believe It is. Herbert, please correct me if I am wrong.
  A single tfm is used as a user context to crypto, so to speak. But a user 
  is not a thread.
  Let us use ipsec as example.
  For each security association (SA), it will take up a tfm.
  Assume I have IP sec setup between my local host and remote host.  I might 
  have two SA's, one for each direction.
  Now, I might run ping. Simultaneously, I might run iperf. I might run a lot 
  of different things  between these two ip hosts.
  But only two tfm's are involved.
  I have seen this happening   in our system with ipsec setup as described 
  above.
  While an  async request is outstanding in the driver,  another request is  
  issued to the same driver for the same tfm.
 
 Yes you're absolutely right.
 
 Unless I've misunderstood Marcelo's question is different from
 what Garg was asking.
 
 Marcelo: The tfm, be it blkcipher or ablkcipher can always be used
 in parallel by the user on different CPUs.  For example, IPsec may
 receive two packets on two CPUs through the same SA, in which case
 decryption will be carried out in parallel.

So does that means that it's possible to keep data in the tfm's context
that is the same for a single SA, such as the AES expanded key, but it's
not possible to keep data that is specific for the current operation,
such as an operation state that the driver might require?

Actually I think that probably I have misunderstood the blkcipher
interface, so here it is another question: is each encrypt/decrypt call
a complete operation? I mean, I'm considering that I could always chain
a series of calls to encrypt data in separated chunks, in a similar way
that is done for the hash interface and because that I'm assuming that I
would have to keep state between those calls if the device requires
that.

 
 Garg: For any tfm, blkcipher or ablkcipher, they must return results
 in the order they were given.  For a blkcipher which is synchronous,
 this is always true by definition since we return only after the
 result has been completed.  For an async ablkcipher, this means that
 if you receive two requests on the same CPU, then the first request
 must be served and completed before the second request's completion
 can be initiated.
 
 Sorry for any confusion this might have caused.
 
 Cheers,
 -- 
 Email: Herbert Xu herb...@gondor.apana.org.au
 Home Page: http://gondor.apana.org.au/~herbert/
 PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
 --
 To unsubscribe from this list: send the line unsubscribe linux-crypto in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] crypto: nx - fix concurrency issue

[Marcelo Cerri]

The NX driver uses the transformation context to store several fields
containing data related to the state of the operations in progress.
Since a single tfm can be used by different kernel threads at the same
time, we need to protect the data stored into the context.

This patch makes use of spin locks to protect the data where a race
condition can happen.

Reviewed-by: Fionnuala Gunter f...@linux.vnet.ibm.com
Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-aes-cbc.c  | 10 --
 drivers/crypto/nx/nx-aes-ccm.c  | 20 
 drivers/crypto/nx/nx-aes-ctr.c  | 10 --
 drivers/crypto/nx/nx-aes-ecb.c  | 10 --
 drivers/crypto/nx/nx-aes-gcm.c  |  4 
 drivers/crypto/nx/nx-aes-xcbc.c |  8 
 drivers/crypto/nx/nx-sha256.c   | 16 
 drivers/crypto/nx/nx-sha512.c   | 16 
 drivers/crypto/nx/nx.c  |  4 ++--
 drivers/crypto/nx/nx.h  |  1 +
 10 files changed, 87 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c
index 6d90808..9310982 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -70,10 +70,15 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc *desc,
 {
struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc-tfm);
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
+   unsigned long irq_flags;
int rc;
 
-   if (nbytes  nx_ctx-ap-databytelen)
-   return -EINVAL;
+   spin_lock_irqsave(nx_ctx-lock, irq_flags);
+
+   if (nbytes  nx_ctx-ap-databytelen) {
+   rc = -EINVAL;
+   goto out;
+   }
 
if (enc)
NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
@@ -101,6 +106,7 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc *desc,
atomic64_add(csbcpb-csb.processed_byte_count,
 (nx_ctx-stats-aes_bytes));
 out:
+   spin_unlock_irqrestore(nx_ctx-lock, irq_flags);
return rc;
 }
 
diff --git a/drivers/crypto/nx/nx-aes-ccm.c b/drivers/crypto/nx/nx-aes-ccm.c
index ef5eae6..39d4224 100644
--- a/drivers/crypto/nx/nx-aes-ccm.c
+++ b/drivers/crypto/nx/nx-aes-ccm.c
@@ -271,10 +271,15 @@ static int ccm_nx_decrypt(struct aead_request   *req,
unsigned int nbytes = req-cryptlen;
unsigned int authsize = crypto_aead_authsize(crypto_aead_reqtfm(req));
struct nx_ccm_priv *priv = nx_ctx-priv.ccm;
+   unsigned long irq_flags;
int rc = -1;
 
-   if (nbytes  nx_ctx-ap-databytelen)
-   return -EINVAL;
+   spin_lock_irqsave(nx_ctx-lock, irq_flags);
+
+   if (nbytes  nx_ctx-ap-databytelen) {
+   rc = -EINVAL;
+   goto out;
+   }
 
nbytes -= authsize;
 
@@ -308,6 +313,7 @@ static int ccm_nx_decrypt(struct aead_request   *req,
rc = memcmp(csbcpb-cpb.aes_ccm.out_pat_or_mac, priv-oauth_tag,
authsize) ? -EBADMSG : 0;
 out:
+   spin_unlock_irqrestore(nx_ctx-lock, irq_flags);
return rc;
 }
 
@@ -318,10 +324,15 @@ static int ccm_nx_encrypt(struct aead_request   *req,
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
unsigned int nbytes = req-cryptlen;
unsigned int authsize = crypto_aead_authsize(crypto_aead_reqtfm(req));
+   unsigned long irq_flags;
int rc = -1;
 
-   if (nbytes  nx_ctx-ap-databytelen)
-   return -EINVAL;
+   spin_lock_irqsave(nx_ctx-lock, irq_flags);
+
+   if (nbytes  nx_ctx-ap-databytelen) {
+   rc = -EINVAL;
+   goto out;
+   }
 
rc = generate_pat(desc-info, req, nx_ctx, authsize, nbytes,
  csbcpb-cpb.aes_ccm.in_pat_or_b0);
@@ -350,6 +361,7 @@ static int ccm_nx_encrypt(struct aead_request   *req,
 req-dst, nbytes, authsize,
 SCATTERWALK_TO_SG);
 out:
+   spin_unlock_irqrestore(nx_ctx-lock, irq_flags);
return rc;
 }
 
diff --git a/drivers/crypto/nx/nx-aes-ctr.c b/drivers/crypto/nx/nx-aes-ctr.c
index b6286f1..762611b 100644
--- a/drivers/crypto/nx/nx-aes-ctr.c
+++ b/drivers/crypto/nx/nx-aes-ctr.c
@@ -88,10 +88,15 @@ static int ctr_aes_nx_crypt(struct blkcipher_desc *desc,
 {
struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc-tfm);
struct nx_csbcpb *csbcpb = nx_ctx-csbcpb;
+   unsigned long irq_flags;
int rc;
 
-   if (nbytes  nx_ctx-ap-databytelen)
-   return -EINVAL;
+   spin_lock_irqsave(nx_ctx-lock, irq_flags);
+
+   if (nbytes  nx_ctx-ap-databytelen) {
+   rc = -EINVAL;
+   goto out;
+   }
 
rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes,
   csbcpb-cpb.aes_ctr.iv);
@@ -112,6 +117,7 @@ static int ctr_aes_nx_crypt(struct blkcipher_desc *desc,
atomic64_add(csbcpb-csb.processed_byte_count

Re: Questions about the Crypto API

[Marcelo Cerri]

Hi Herbert,

Thanks for your answers.

On Tue, Aug 06, 2013 at 05:00:10PM +1000, Herbert Xu wrote:
 On Mon, Aug 05, 2013 at 05:25:57PM -0300, Marcelo Cerri wrote:
 
  My first doubt is regarding which kind of concurrency the Crypto API
  allows. For example, can a single `struct crypto_tfm` be used by two
  concurrent calls? I'm asking about that because I noticed that for
 
 Yes.
 
  blkcipher the only implementation-specific context that can be used is
  allocated inside the tfm struct.
 
 Both blkcipher and ablkcipher are meant to be fully reentrant.
 So you must take necessary precautions if your implementation
 is not reentrant, e.g., by locking.

I was considering a spin lock for that, since the cryptographic
functions can be called from a softirq context. However I don't consider
a lock a good solution because that could be done without any locks if
it was possible to keep the current state separated for each operation
in progress.

 
  I'm working to fix some bugs in the NX driver (located in
  drivers/crypto/nx), and one issue that we are facing is that NFS when
  using Kerberos uses the same tfm with different kthreads. That causes
  concurrent accesses to the internal data stored into the context and
  incorrect results.
  
  So my question here is: should this type of concurrency be handled by
  the driver or a caller is not allowed to use the same tfm for concurrent
  calls?
 
 From what you've said NFS seems to be doing the right thing, so the
 bug would be in the driver.
 
  My second doubt is regarding the difference between ablkcipher and
  blkcipher. I do understand their difference from caller's point of view.
  But I'm not sure what are the consequences of implementing a driver
  using one or another option.
  
  For example, can a blkcipher implementation be used asynchronously and
  vice versa?
 
 In general which model you pick for drivers depend on what your
 hardware looks like.  For example, padlock-aes uses the blkcipher
 model because the hardware presents itself through a synchronous
 CPU instruction, while most other drivers use the ablkcipher
 interface because the underlying hardware completes asynchronously.
 
 A blkcipher implementation is always available through both the
 blkcipher and the ablkcipher interface.  While an ablkcipher
 implementaiton can only be used through the ablkcipher interface.

Now a lot of things start to make sense :P

So is that the reason because some drivers implement an ablkcipher and
then re-implements the same algorithm as a blkcipher just using a wrapper
over the asynchronous version?

I saw it's possible to keep a context in an ablkcipher_request
structure. I'm assuming that multiple callers using the same tfm still
would have to use different requests. So do you think that implementing
it as an asynchronous block cipher would be an alternative to locks in
the NX driver?

Regards,
Marcelo

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Questions about the Crypto API

[Marcelo Cerri]

Herbert,

Let me include a few more questions.

There are flags in several structures such as crypto_async_request,
blkcipher_desc and crypto_tfm. How they are intended to be used?

For example if I want to explicitly make a call that shouldn't sleep,
should I clear the CRYPTO_TFM_REQ_MAY_SLEEP in one of these structures?
And in which one?

Since cryptographic methods can be called in softirq contexts, is the
caller responsible for setting this flag correctly based on the context
it is running?

Regards,
Marcelo

On Tue, Aug 06, 2013 at 09:05:41AM -0300, Marcelo Cerri wrote:
 Hi Herbert,
 
 Thanks for your answers.
 
 On Tue, Aug 06, 2013 at 05:00:10PM +1000, Herbert Xu wrote:
  On Mon, Aug 05, 2013 at 05:25:57PM -0300, Marcelo Cerri wrote:
  
   My first doubt is regarding which kind of concurrency the Crypto API
   allows. For example, can a single `struct crypto_tfm` be used by two
   concurrent calls? I'm asking about that because I noticed that for
  
  Yes.
  
   blkcipher the only implementation-specific context that can be used is
   allocated inside the tfm struct.
  
  Both blkcipher and ablkcipher are meant to be fully reentrant.
  So you must take necessary precautions if your implementation
  is not reentrant, e.g., by locking.
 
 I was considering a spin lock for that, since the cryptographic
 functions can be called from a softirq context. However I don't consider
 a lock a good solution because that could be done without any locks if
 it was possible to keep the current state separated for each operation
 in progress.
 
  
   I'm working to fix some bugs in the NX driver (located in
   drivers/crypto/nx), and one issue that we are facing is that NFS when
   using Kerberos uses the same tfm with different kthreads. That causes
   concurrent accesses to the internal data stored into the context and
   incorrect results.
   
   So my question here is: should this type of concurrency be handled by
   the driver or a caller is not allowed to use the same tfm for concurrent
   calls?
  
  From what you've said NFS seems to be doing the right thing, so the
  bug would be in the driver.
  
   My second doubt is regarding the difference between ablkcipher and
   blkcipher. I do understand their difference from caller's point of view.
   But I'm not sure what are the consequences of implementing a driver
   using one or another option.
   
   For example, can a blkcipher implementation be used asynchronously and
   vice versa?
  
  In general which model you pick for drivers depend on what your
  hardware looks like.  For example, padlock-aes uses the blkcipher
  model because the hardware presents itself through a synchronous
  CPU instruction, while most other drivers use the ablkcipher
  interface because the underlying hardware completes asynchronously.
  
  A blkcipher implementation is always available through both the
  blkcipher and the ablkcipher interface.  While an ablkcipher
  implementaiton can only be used through the ablkcipher interface.
 
 Now a lot of things start to make sense :P
 
 So is that the reason because some drivers implement an ablkcipher and
 then re-implements the same algorithm as a blkcipher just using a wrapper
 over the asynchronous version?
 
 I saw it's possible to keep a context in an ablkcipher_request
 structure. I'm assuming that multiple callers using the same tfm still
 would have to use different requests. So do you think that implementing
 it as an asynchronous block cipher would be an alternative to locks in
 the NX driver?
 
 Regards,
 Marcelo
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-crypto in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Questions about the Crypto API

[Marcelo Cerri]

Hi,

I'm starting to work on some platform-specific implementations using the
Crypto API. I spent some time reading the available documentation and
mainly the code, but I still have some doubts on how the Crypto API
works and how it should be used.

My first doubt is regarding which kind of concurrency the Crypto API
allows. For example, can a single `struct crypto_tfm` be used by two
concurrent calls? I'm asking about that because I noticed that for
blkcipher the only implementation-specific context that can be used is
allocated inside the tfm struct.

I'm working to fix some bugs in the NX driver (located in
drivers/crypto/nx), and one issue that we are facing is that NFS when
using Kerberos uses the same tfm with different kthreads. That causes
concurrent accesses to the internal data stored into the context and
incorrect results.

So my question here is: should this type of concurrency be handled by
the driver or a caller is not allowed to use the same tfm for concurrent
calls?

My second doubt is regarding the difference between ablkcipher and
blkcipher. I do understand their difference from caller's point of view.
But I'm not sure what are the consequences of implementing a driver
using one or another option.

For example, can a blkcipher implementation be used asynchronously and
vice versa?

Thanks for your help.
Marcelo

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/2] drivers/crypto/nx: fixes when input data is too large

[Marcelo Cerri]

This series of patches fixes two bugs that are triggered when the input data is
too large. The first one is caused by the miscalculation of physical addresses
and the second one by some limits that the co-processor has to the input data.

Changes in v2:
* Replace Signed-Off-By tags with Reviewed-By tags where it is
  appropriate.

Marcelo Cerri (2):
  drivers/crypto/nx: fix physical addresses added to sg lists
  drivers/crypto/nx: fix limits to sg lists for SHA-2

 drivers/crypto/nx/nx-sha256.c | 108 +++-
 drivers/crypto/nx/nx-sha512.c | 113 --
 drivers/crypto/nx/nx.c|  22 ++--
 3 files changed, 148 insertions(+), 95 deletions(-)

-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] drivers/crypto/nx: fix limits to sg lists for SHA-2

[Marcelo Cerri]

The co-processor has several limits regarding the length of
scatter/gather lists and the total number of bytes in it. These limits
are available in the device tree, as following:

 - ibm,max-sg-len: maximum number of bytes of each scatter/gather
   list.

 - ibm,max-sync-cop: used for synchronous operations, it is an array
   of structures that contains information regarding the limits that
   must be considered for each mode and operation. The most important
   limits in it are:
- The total number of bytes that a scatter/gather list can hold.
- The maximum number of elements that a scatter/gather list can
  have.

This patch updates the NX driver to perform several hyper calls if
needed in order to always respect the length limits for scatter/gather
lists.

Reviewed-by: Fionnuala Gunter f...@linux.vnet.ibm.com
Reviewed-by: Joel Schopp jsch...@linux.vnet.ibm.com
Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-sha256.c | 108 +++-
 drivers/crypto/nx/nx-sha512.c | 113 --
 2 files changed, 129 insertions(+), 92 deletions(-)

diff --git a/drivers/crypto/nx/nx-sha256.c b/drivers/crypto/nx/nx-sha256.c
index 67024f2..254b01a 100644
--- a/drivers/crypto/nx/nx-sha256.c
+++ b/drivers/crypto/nx/nx-sha256.c
@@ -55,70 +55,86 @@ static int nx_sha256_update(struct shash_desc *desc, const 
u8 *data,
struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(desc-tfm-base);
struct nx_csbcpb *csbcpb = (struct nx_csbcpb *)nx_ctx-csbcpb;
struct nx_sg *in_sg;
-   u64 to_process, leftover;
+   u64 to_process, leftover, total;
+   u32 max_sg_len;
int rc = 0;
 
-   if (NX_CPB_FDM(csbcpb)  NX_FDM_CONTINUATION) {
-   /* we've hit the nx chip previously and we're updating again,
-* so copy over the partial digest */
-   memcpy(csbcpb-cpb.sha256.input_partial_digest,
-  csbcpb-cpb.sha256.message_digest, SHA256_DIGEST_SIZE);
-   }
-
/* 2 cases for total data len:
-*  1: = SHA256_BLOCK_SIZE: copy into state, return 0
-*  2:  SHA256_BLOCK_SIZE: process X blocks, copy in leftover
+*  1:  SHA256_BLOCK_SIZE: copy into state, return 0
+*  2: = SHA256_BLOCK_SIZE: process X blocks, copy in leftover
 */
-   if (len + sctx-count  SHA256_BLOCK_SIZE) {
+   total = sctx-count + len;
+   if (total  SHA256_BLOCK_SIZE) {
memcpy(sctx-buf + sctx-count, data, len);
sctx-count += len;
goto out;
}
 
-   /* to_process: the SHA256_BLOCK_SIZE data chunk to process in this
-* update */
-   to_process = (sctx-count + len)  ~(SHA256_BLOCK_SIZE - 1);
-   leftover = (sctx-count + len)  (SHA256_BLOCK_SIZE - 1);
+   in_sg = nx_ctx-in_sg;
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+  nx_ctx-ap-sglen);
 
-   if (sctx-count) {
-   in_sg = nx_build_sg_list(nx_ctx-in_sg, (u8 *)sctx-buf,
-sctx-count, nx_ctx-ap-sglen);
-   in_sg = nx_build_sg_list(in_sg, (u8 *)data,
+   do {
+   /*
+* to_process: the SHA256_BLOCK_SIZE data chunk to process in
+* this update. This value is also restricted by the sg list
+* limits.
+*/
+   to_process = min_t(u64, total, nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+  NX_PAGE_SIZE * (max_sg_len - 1));
+   to_process = to_process  ~(SHA256_BLOCK_SIZE - 1);
+   leftover = total - to_process;
+
+   if (sctx-count) {
+   in_sg = nx_build_sg_list(nx_ctx-in_sg,
+(u8 *) sctx-buf,
+sctx-count, max_sg_len);
+   }
+   in_sg = nx_build_sg_list(in_sg, (u8 *) data,
 to_process - sctx-count,
-nx_ctx-ap-sglen);
+max_sg_len);
nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) *
sizeof(struct nx_sg);
-   } else {
-   in_sg = nx_build_sg_list(nx_ctx-in_sg, (u8 *)data,
-to_process, nx_ctx-ap-sglen);
-   nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) *
-   sizeof(struct nx_sg);
-   }
 
-   NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+   if (NX_CPB_FDM(csbcpb)  NX_FDM_CONTINUATION) {
+   /*
+* we've hit the nx chip previously and we're updating

[PATCH v2 1/2] drivers/crypto/nx: fix physical addresses added to sg lists

[Marcelo Cerri]

The co-processor receives data to be hashed through scatter/gather lists
pointing to physical addresses. When a vmalloc'ed data is given, the
driver must calculate the physical address to each page of the data.

However the current version of it just calculates the physical address
once and keeps incrementing it even when a page boundary is crossed.
This patch fixes this behaviour.

Reviewed-by: Fionnuala Gunter f...@linux.vnet.ibm.com
Reviewed-by: Joel Schopp jsch...@linux.vnet.ibm.com
Reviewed-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c
index bbdab6e..ad07dc6 100644
--- a/drivers/crypto/nx/nx.c
+++ b/drivers/crypto/nx/nx.c
@@ -114,13 +114,29 @@ struct nx_sg *nx_build_sg_list(struct nx_sg *sg_head,
 * have been described (or @sgmax elements have been written), the
 * loop ends. min_t is used to ensure @end_addr falls on the same page
 * as sg_addr, if not, we need to create another nx_sg element for the
-* data on the next page */
+* data on the next page.
+*
+* Also when using vmalloc'ed data, every time that a system page
+* boundary is crossed the physical address needs to be re-calculated.
+*/
for (sg = sg_head; sg_len  len; sg++) {
+   u64 next_page;
+
sg-addr = sg_addr;
-   sg_addr = min_t(u64, NX_PAGE_NUM(sg_addr + NX_PAGE_SIZE), 
end_addr);
-   sg-len = sg_addr - sg-addr;
+   sg_addr = min_t(u64, NX_PAGE_NUM(sg_addr + NX_PAGE_SIZE),
+   end_addr);
+
+   next_page = (sg-addr  PAGE_MASK) + PAGE_SIZE;
+   sg-len = min_t(u64, sg_addr, next_page) - sg-addr;
sg_len += sg-len;
 
+   if (sg_addr = next_page 
+   is_vmalloc_addr(start_addr + sg_len)) {
+   sg_addr = page_to_phys(vmalloc_to_page(
+   start_addr + sg_len));
+   end_addr = sg_addr + len - sg_len;
+   }
+
if ((sg - sg_head) == sgmax) {
pr_err(nx: scatter/gather list overflow, pid: %d\n,
   current-pid);
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] drivers/crypto/nx: fix limits to sg lists for SHA-2

[Marcelo Cerri]

We think that it's very likely that AES may also be affected by a
similar problem. But we still have to test it and I'd like to provide a
separated patch for it.

Regards,
Marcelo

On Sat, Jul 27, 2013 at 08:31:32AM +1000, Benjamin Herrenschmidt wrote:
 On Fri, 2013-07-26 at 14:08 -0300, Marcelo Cerri wrote:
  ---
   drivers/crypto/nx/nx-sha256.c | 108 
  +++-
   drivers/crypto/nx/nx-sha512.c | 113 
  --
   2 files changed, 129 insertions(+), 92 deletions(-)
 
 What about the other nx drivers ? They are not affected ?
 
 Cheers,
 Ben.
 
 

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] drivers/crypto/nx: fix limits to sg lists for SHA-2

[Marcelo Cerri]

Hi Ben,

Everyone in S-O-B list has participated to solve this bug with code
and/or ideas of how to fix it, as well as reviewing and testing the
final version of the patches.

I'd like to keep it as it is if you don't mind.

Regards.
Marcelo

On Sat, Jul 27, 2013 at 08:29:59AM +1000, Benjamin Herrenschmidt wrote:
 On Fri, 2013-07-26 at 14:08 -0300, Marcelo Cerri wrote:
  
  Signed-off-by: Fionnuala Gunter f...@linux.vnet.ibm.com
  Signed-off-by: Joel Schopp jsch...@linux.vnet.ibm.com
  Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
  Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
  ---
 
 Why that enormous S-O-B list ? Did every of these people actually carry
 the patch ? If it's just acks or reviews, please use the corresponding
 Acked-by or Reviewed-by.
 
 Cheers,
 Ben.
 
 

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] drivers/crypto/nx: fixes when input data is too large

[Marcelo Cerri]

This series of patches fixes two bugs that are triggered when the input data is
too large. The first one is caused by the miscalculation of physical addresses
and the second one by some limits that the co-processor has to the input data.

Marcelo Cerri (2):
  drivers/crypto/nx: fix physical addresses added to sg lists
  drivers/crypto/nx: fix limits to sg lists for SHA-2

 drivers/crypto/nx/nx-sha256.c | 108 +++-
 drivers/crypto/nx/nx-sha512.c | 113 --
 drivers/crypto/nx/nx.c|  22 ++--
 3 files changed, 148 insertions(+), 95 deletions(-)

-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] drivers/crypto/nx: fix physical addresses added to sg lists

[Marcelo Cerri]

The co-processor receives data to be hashed through scatter/gather lists
pointing to physical addresses. When a vmalloc'ed data is given, the
driver must calculate the physical address to each page of the data.

However the current version of it just calculates the physical address
once and keeps incrementing it even when a page boundary is crossed.
This patch fixes this behaviour.

Signed-off-by: Fionnuala Gunter f...@linux.vnet.ibm.com
Signed-off-by: Joel Schopp jsch...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c
index bbdab6e..ad07dc6 100644
--- a/drivers/crypto/nx/nx.c
+++ b/drivers/crypto/nx/nx.c
@@ -114,13 +114,29 @@ struct nx_sg *nx_build_sg_list(struct nx_sg *sg_head,
 * have been described (or @sgmax elements have been written), the
 * loop ends. min_t is used to ensure @end_addr falls on the same page
 * as sg_addr, if not, we need to create another nx_sg element for the
-* data on the next page */
+* data on the next page.
+*
+* Also when using vmalloc'ed data, every time that a system page
+* boundary is crossed the physical address needs to be re-calculated.
+*/
for (sg = sg_head; sg_len  len; sg++) {
+   u64 next_page;
+
sg-addr = sg_addr;
-   sg_addr = min_t(u64, NX_PAGE_NUM(sg_addr + NX_PAGE_SIZE), 
end_addr);
-   sg-len = sg_addr - sg-addr;
+   sg_addr = min_t(u64, NX_PAGE_NUM(sg_addr + NX_PAGE_SIZE),
+   end_addr);
+
+   next_page = (sg-addr  PAGE_MASK) + PAGE_SIZE;
+   sg-len = min_t(u64, sg_addr, next_page) - sg-addr;
sg_len += sg-len;
 
+   if (sg_addr = next_page 
+   is_vmalloc_addr(start_addr + sg_len)) {
+   sg_addr = page_to_phys(vmalloc_to_page(
+   start_addr + sg_len));
+   end_addr = sg_addr + len - sg_len;
+   }
+
if ((sg - sg_head) == sgmax) {
pr_err(nx: scatter/gather list overflow, pid: %d\n,
   current-pid);
-- 
1.7.12

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] drivers/crypto/nx: fix limits to sg lists for SHA-2

[Marcelo Cerri]

The co-processor has several limits regarding the length of
scatter/gather lists and the total number of bytes in it. These limits
are available in the device tree, as following:

 - ibm,max-sg-len: maximum number of bytes of each scatter/gather
   list.

 - ibm,max-sync-cop: used for synchronous operations, it is an array
   of structures that contains information regarding the limits that
   must be considered for each mode and operation. The most important
   limits in it are:
- The total number of bytes that a scatter/gather list can hold.
- The maximum number of elements that a scatter/gather list can
  have.

This patch updates the NX driver to perform several hyper calls if
needed in order to always respect the length limits for scatter/gather
lists.

Signed-off-by: Fionnuala Gunter f...@linux.vnet.ibm.com
Signed-off-by: Joel Schopp jsch...@linux.vnet.ibm.com
Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com
Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
---
 drivers/crypto/nx/nx-sha256.c | 108 +++-
 drivers/crypto/nx/nx-sha512.c | 113 --
 2 files changed, 129 insertions(+), 92 deletions(-)

diff --git a/drivers/crypto/nx/nx-sha256.c b/drivers/crypto/nx/nx-sha256.c
index 67024f2..254b01a 100644
--- a/drivers/crypto/nx/nx-sha256.c
+++ b/drivers/crypto/nx/nx-sha256.c
@@ -55,70 +55,86 @@ static int nx_sha256_update(struct shash_desc *desc, const 
u8 *data,
struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(desc-tfm-base);
struct nx_csbcpb *csbcpb = (struct nx_csbcpb *)nx_ctx-csbcpb;
struct nx_sg *in_sg;
-   u64 to_process, leftover;
+   u64 to_process, leftover, total;
+   u32 max_sg_len;
int rc = 0;
 
-   if (NX_CPB_FDM(csbcpb)  NX_FDM_CONTINUATION) {
-   /* we've hit the nx chip previously and we're updating again,
-* so copy over the partial digest */
-   memcpy(csbcpb-cpb.sha256.input_partial_digest,
-  csbcpb-cpb.sha256.message_digest, SHA256_DIGEST_SIZE);
-   }
-
/* 2 cases for total data len:
-*  1: = SHA256_BLOCK_SIZE: copy into state, return 0
-*  2:  SHA256_BLOCK_SIZE: process X blocks, copy in leftover
+*  1:  SHA256_BLOCK_SIZE: copy into state, return 0
+*  2: = SHA256_BLOCK_SIZE: process X blocks, copy in leftover
 */
-   if (len + sctx-count  SHA256_BLOCK_SIZE) {
+   total = sctx-count + len;
+   if (total  SHA256_BLOCK_SIZE) {
memcpy(sctx-buf + sctx-count, data, len);
sctx-count += len;
goto out;
}
 
-   /* to_process: the SHA256_BLOCK_SIZE data chunk to process in this
-* update */
-   to_process = (sctx-count + len)  ~(SHA256_BLOCK_SIZE - 1);
-   leftover = (sctx-count + len)  (SHA256_BLOCK_SIZE - 1);
+   in_sg = nx_ctx-in_sg;
+   max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+  nx_ctx-ap-sglen);
 
-   if (sctx-count) {
-   in_sg = nx_build_sg_list(nx_ctx-in_sg, (u8 *)sctx-buf,
-sctx-count, nx_ctx-ap-sglen);
-   in_sg = nx_build_sg_list(in_sg, (u8 *)data,
+   do {
+   /*
+* to_process: the SHA256_BLOCK_SIZE data chunk to process in
+* this update. This value is also restricted by the sg list
+* limits.
+*/
+   to_process = min_t(u64, total, nx_ctx-ap-databytelen);
+   to_process = min_t(u64, to_process,
+  NX_PAGE_SIZE * (max_sg_len - 1));
+   to_process = to_process  ~(SHA256_BLOCK_SIZE - 1);
+   leftover = total - to_process;
+
+   if (sctx-count) {
+   in_sg = nx_build_sg_list(nx_ctx-in_sg,
+(u8 *) sctx-buf,
+sctx-count, max_sg_len);
+   }
+   in_sg = nx_build_sg_list(in_sg, (u8 *) data,
 to_process - sctx-count,
-nx_ctx-ap-sglen);
+max_sg_len);
nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) *
sizeof(struct nx_sg);
-   } else {
-   in_sg = nx_build_sg_list(nx_ctx-in_sg, (u8 *)data,
-to_process, nx_ctx-ap-sglen);
-   nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) *
-   sizeof(struct nx_sg);
-   }
 
-   NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+   if (NX_CPB_FDM(csbcpb)  NX_FDM_CONTINUATION) {
+   /*
+* we've hit the nx chip previously and we're updating