2013-08-07 23:15:50

by Fionnuala Gunter

[permalink] [raw]
Subject: [PATCH v2] drivers/crypto/nx: saves chaining value from co-processor

This patch fixes a bug that is triggered when cts(cbc(aes)) is used with
nx-crypto driver on input larger than 32 bytes.

The chaining value from co-processor was not being saved. This value is
needed because it is used as the IV by cts(cbc(aes)).

Signed-off-by: Fionnuala Gunter <[email protected]>
Reviewed-by: Marcelo Cerri <[email protected]>
---
v2. changed signed-off-by to reviewed-by and added more details to
description

This bug appeared in the original submission (v3.5)
---
drivers/crypto/nx/nx-aes-cbc.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/nx/nx-aes-cbc.c
b/drivers/crypto/nx/nx-aes-cbc.c
index 35d483f..a2f99a9 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -95,6 +95,7 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc
*desc,
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));
--
1.7.10.4


2013-08-08 07:49:25

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/crypto/nx: saves chaining value from co-processor

Hi Fin,

I don't know anything about crypto so I can only critique you on your
patch submission technique :) ...

On Wed, Aug 07, 2013 at 06:15:50PM -0500, Fionnuala Gunter wrote:
> This patch fixes a bug that is triggered when cts(cbc(aes)) is used with
> nx-crypto driver on input larger than 32 bytes.
>
> The chaining value from co-processor was not being saved. This value is
> needed because it is used as the IV by cts(cbc(aes)).
>
> Signed-off-by: Fionnuala Gunter <[email protected]>
> Reviewed-by: Marcelo Cerri <[email protected]>
> ---
> v2. changed signed-off-by to reviewed-by and added more details to
> description
>
> This bug appeared in the original submission (v3.5)

Ideally this should identify the commit, so:

This bug was introduced in the original submission (v3.5), commit
856d673 "powerpc/crypto: AES-CBC mode routines for nx encryption".

Including the subject of the commit is handy in case the patch has been
backported somewhere, in which case the commit sha will be different.

It should definitely be part of the commit message, not below the "---".

And Ben might disagree but I think with a clear cut bug fix like this it
should include the CC to stable, so:

Cc: [email protected] # 3.5+

cheers

2013-08-09 06:25:23

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/crypto/nx: saves chaining value from co-processor

On Wed, 2013-08-07 at 18:15 -0500, Fionnuala Gunter wrote:
> This patch fixes a bug that is triggered when cts(cbc(aes)) is used with
> nx-crypto driver on input larger than 32 bytes.
>
> The chaining value from co-processor was not being saved. This value is
> needed because it is used as the IV by cts(cbc(aes)).
>
> Signed-off-by: Fionnuala Gunter <[email protected]>
> Reviewed-by: Marcelo Cerri <[email protected]>

Herbert, I assume you will handle this along with all the other NX fixes
and I can safely take them out of linuxppc patchwork ?

Cheers,
Ben.

> ---
> v2. changed signed-off-by to reviewed-by and added more details to
> description
>
> This bug appeared in the original submission (v3.5)
> ---
> drivers/crypto/nx/nx-aes-cbc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/crypto/nx/nx-aes-cbc.c
> b/drivers/crypto/nx/nx-aes-cbc.c
> index 35d483f..a2f99a9 100644
> --- a/drivers/crypto/nx/nx-aes-cbc.c
> +++ b/drivers/crypto/nx/nx-aes-cbc.c
> @@ -95,6 +95,7 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc
> *desc,
> 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));

2013-08-09 06:30:12

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/crypto/nx: saves chaining value from co-processor

On Fri, Aug 09, 2013 at 04:25:04PM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2013-08-07 at 18:15 -0500, Fionnuala Gunter wrote:
> > This patch fixes a bug that is triggered when cts(cbc(aes)) is used with
> > nx-crypto driver on input larger than 32 bytes.
> >
> > The chaining value from co-processor was not being saved. This value is
> > needed because it is used as the IV by cts(cbc(aes)).
> >
> > Signed-off-by: Fionnuala Gunter <[email protected]>
> > Reviewed-by: Marcelo Cerri <[email protected]>
>
> Herbert, I assume you will handle this along with all the other NX fixes
> and I can safely take them out of linuxppc patchwork ?

Yes of course.

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

2013-08-09 06:43:37

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/crypto/nx: saves chaining value from co-processor

On Wed, Aug 07, 2013 at 06:15:50PM -0500, Fionnuala Gunter wrote:
> This patch fixes a bug that is triggered when cts(cbc(aes)) is used with
> nx-crypto driver on input larger than 32 bytes.
>
> The chaining value from co-processor was not being saved. This value is
> needed because it is used as the IV by cts(cbc(aes)).
>
> Signed-off-by: Fionnuala Gunter <[email protected]>
> Reviewed-by: Marcelo Cerri <[email protected]>
> ---
> v2. changed signed-off-by to reviewed-by and added more details to
> description
>
> This bug appeared in the original submission (v3.5)

Patch applied. However your patch was totally white-space damaged.
I've fixed it this time, but please fix this in your future
submissions.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt