diff mbox

[v2] drivers/crypto/nx: saves chaining value from co-processor

Message ID 1375917350.16050.14.camel@what-is-leg (mailing list archive)
State Not Applicable
Headers show

Commit Message

Fionnuala Gunter Aug. 7, 2013, 11:15 p.m. UTC
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 <fin@linux.vnet.ibm.com>
Reviewed-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
---
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(+)

                     &(nx_ctx->stats->aes_bytes));

Comments

Michael Ellerman Aug. 8, 2013, 7:49 a.m. UTC | #1
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 <fin@linux.vnet.ibm.com>
> Reviewed-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
> ---
> 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: stable@vger.kernel.org # 3.5+

cheers
Benjamin Herrenschmidt Aug. 9, 2013, 6:25 a.m. UTC | #2
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 <fin@linux.vnet.ibm.com>
> Reviewed-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>

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));
Herbert Xu Aug. 9, 2013, 6:30 a.m. UTC | #3
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 <fin@linux.vnet.ibm.com>
> > Reviewed-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
> 
> 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,
Herbert Xu Aug. 9, 2013, 6:43 a.m. UTC | #4
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 <fin@linux.vnet.ibm.com>
> Reviewed-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
> ---
> 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.
diff mbox

Patch

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,