2019-11-08 08:58:11

by Pascal van Leeuwen

Subject: [PATCHv3] crypto: inside-secure - Fixed authenc w/ (3)DES fails on Macchiatobin

Fixed 2 copy-paste mistakes in the commit mentioned below that caused
authenc w/ (3)DES to consistently fail on Macchiatobin (but strangely
work fine on x86+FPGA??).
Now fully tested on both platforms.

changes since v1:
- added Fixes: tag

changes since v2:
- moved Fixes: tag down near other tags

Fixes: 13a1bb93f7b1c9 ("crypto: inside-secure - Fixed warnings on
inconsistent byte order handling")

Signed-off-by: Pascal van Leeuwen <[email protected]>
---
drivers/crypto/inside-secure/safexcel_cipher.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index 98f9fc6..c029956 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -405,7 +405,8 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,

if (priv->flags & EIP197_TRC_CACHE && ctx->base.ctxr_dma) {
for (i = 0; i < keys.enckeylen / sizeof(u32); i++) {
- if (le32_to_cpu(ctx->key[i]) != aes.key_enc[i]) {
+ if (le32_to_cpu(ctx->key[i]) !=
+ ((u32 *)keys.enckey)[i]) {
ctx->base.needs_inv = true;
break;
}
@@ -459,7 +460,7 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,

/* Now copy the keys into the context */
for (i = 0; i < keys.enckeylen / sizeof(u32); i++)
- ctx->key[i] = cpu_to_le32(aes.key_enc[i]);
+ ctx->key[i] = cpu_to_le32(((u32 *)keys.enckey)[i]);
ctx->key_len = keys.enckeylen;

memcpy(ctx->ipad, &istate.state, ctx->state_sz);
--
1.8.3.1

2019-11-08 09:00:52

by Ard Biesheuvel

Subject: Re: [PATCHv3] crypto: inside-secure - Fixed authenc w/ (3)DES fails on Macchiatobin

On Fri, 8 Nov 2019 at 09:57, Pascal van Leeuwen <[email protected]> wrote:
>
> Fixed 2 copy-paste mistakes in the commit mentioned below that caused
> authenc w/ (3)DES to consistently fail on Macchiatobin (but strangely
> work fine on x86+FPGA??).
> Now fully tested on both platforms.
>
> changes since v1:
> - added Fixes: tag
>
> changes since v2:
> - moved Fixes: tag down near other tags
>

Please put the changelog below the ---
It does not belong in the commit log itself.


> Fixes: 13a1bb93f7b1c9 ("crypto: inside-secure - Fixed warnings on
> inconsistent byte order handling")
>
> Signed-off-by: Pascal van Leeuwen <[email protected]>
> ---
> drivers/crypto/inside-secure/safexcel_cipher.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
> index 98f9fc6..c029956 100644
> --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> @@ -405,7 +405,8 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
>
> if (priv->flags & EIP197_TRC_CACHE && ctx->base.ctxr_dma) {
> for (i = 0; i < keys.enckeylen / sizeof(u32); i++) {
> - if (le32_to_cpu(ctx->key[i]) != aes.key_enc[i]) {
> + if (le32_to_cpu(ctx->key[i]) !=
> + ((u32 *)keys.enckey)[i]) {
> ctx->base.needs_inv = true;
> break;
> }
> @@ -459,7 +460,7 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8 *key,
>
> /* Now copy the keys into the context */
> for (i = 0; i < keys.enckeylen / sizeof(u32); i++)
> - ctx->key[i] = cpu_to_le32(aes.key_enc[i]);
> + ctx->key[i] = cpu_to_le32(((u32 *)keys.enckey)[i]);
> ctx->key_len = keys.enckeylen;
>
> memcpy(ctx->ipad, &istate.state, ctx->state_sz);
> --
> 1.8.3.1
>

2019-11-08 09:03:20

by Pascal Van Leeuwen

Subject: RE: [PATCHv3] crypto: inside-secure - Fixed authenc w/ (3)DES fails on Macchiatobin

> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Friday, November 8, 2019 9:58 AM
> To: Pascal van Leeuwen <[email protected]>
> Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE <[email protected]>;
> Antoine Tenart <[email protected]>; Herbert Xu <[email protected]>;
> David S. Miller <[email protected]>; Pascal Van Leeuwen <[email protected]>
> Subject: Re: [PATCHv3] crypto: inside-secure - Fixed authenc w/ (3)DES fails on
> Macchiatobin
>
> On Fri, 8 Nov 2019 at 09:57, Pascal van Leeuwen <[email protected]> wrote:
> >
> > Fixed 2 copy-paste mistakes in the commit mentioned below that caused
> > authenc w/ (3)DES to consistently fail on Macchiatobin (but strangely
> > work fine on x86+FPGA??).
> > Now fully tested on both platforms.
> >
> > changes since v1:
> > - added Fixes: tag
> >
> > changes since v2:
> > - moved Fixes: tag down near other tags
> >
>
> Please put the changelog below the ---
> It does not belong in the commit log itself.
>
Really? I've always done it like that (just checked some of my previous
patches) and this is the first time someone complains about it ...

>
> > Fixes: 13a1bb93f7b1c9 ("crypto: inside-secure - Fixed warnings on
> > inconsistent byte order handling")
> >
> > Signed-off-by: Pascal van Leeuwen <[email protected]>
> > ---
> > drivers/crypto/inside-secure/safexcel_cipher.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-
> secure/safexcel_cipher.c
> > index 98f9fc6..c029956 100644
> > --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> > +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> > @@ -405,7 +405,8 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8
> *key,
> >
> > if (priv->flags & EIP197_TRC_CACHE && ctx->base.ctxr_dma) {
> > for (i = 0; i < keys.enckeylen / sizeof(u32); i++) {
> > - if (le32_to_cpu(ctx->key[i]) != aes.key_enc[i]) {
> > + if (le32_to_cpu(ctx->key[i]) !=
> > + ((u32 *)keys.enckey)[i]) {
> > ctx->base.needs_inv = true;
> > break;
> > }
> > @@ -459,7 +460,7 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8
> *key,
> >
> > /* Now copy the keys into the context */
> > for (i = 0; i < keys.enckeylen / sizeof(u32); i++)
> > - ctx->key[i] = cpu_to_le32(aes.key_enc[i]);
> > + ctx->key[i] = cpu_to_le32(((u32 *)keys.enckey)[i]);
> > ctx->key_len = keys.enckeylen;
> >
> > memcpy(ctx->ipad, &istate.state, ctx->state_sz);
> > --
> > 1.8.3.1
> >


Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
http://www.insidesecure.com

2019-11-08 09:20:52

by Ard Biesheuvel

Subject: Re: [PATCHv3] crypto: inside-secure - Fixed authenc w/ (3)DES fails on Macchiatobin

On Fri, 8 Nov 2019 at 10:02, Pascal Van Leeuwen
<[email protected]> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <[email protected]>
> > Sent: Friday, November 8, 2019 9:58 AM
> > To: Pascal van Leeuwen <[email protected]>
> > Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE <[email protected]>;
> > Antoine Tenart <[email protected]>; Herbert Xu <[email protected]>;
> > David S. Miller <[email protected]>; Pascal Van Leeuwen <[email protected]>
> > Subject: Re: [PATCHv3] crypto: inside-secure - Fixed authenc w/ (3)DES fails on
> > Macchiatobin
> >
> > On Fri, 8 Nov 2019 at 09:57, Pascal van Leeuwen <[email protected]> wrote:
> > >
> > > Fixed 2 copy-paste mistakes in the commit mentioned below that caused
> > > authenc w/ (3)DES to consistently fail on Macchiatobin (but strangely
> > > work fine on x86+FPGA??).
> > > Now fully tested on both platforms.
> > >
> > > changes since v1:
> > > - added Fixes: tag
> > >
> > > changes since v2:
> > > - moved Fixes: tag down near other tags
> > >
> >
> > Please put the changelog below the ---
> > It does not belong in the commit log itself.
> >
> Really? I've always done it like that (just checked some of my previous
> patches) and this is the first time someone complains about it ...
>

Well, the point is that formatting your patch correctly will make it
easier for the maintainer to apply it, without having to open it in an
editor and move things around or deleting them ('git am' automatically
drops the content below ---)


> >
> > > Fixes: 13a1bb93f7b1c9 ("crypto: inside-secure - Fixed warnings on
> > > inconsistent byte order handling")
> > >
> > > Signed-off-by: Pascal van Leeuwen <[email protected]>
> > > ---
> > > drivers/crypto/inside-secure/safexcel_cipher.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-
> > secure/safexcel_cipher.c
> > > index 98f9fc6..c029956 100644
> > > --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> > > +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> > > @@ -405,7 +405,8 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8
> > *key,
> > >
> > > if (priv->flags & EIP197_TRC_CACHE && ctx->base.ctxr_dma) {
> > > for (i = 0; i < keys.enckeylen / sizeof(u32); i++) {
> > > - if (le32_to_cpu(ctx->key[i]) != aes.key_enc[i]) {
> > > + if (le32_to_cpu(ctx->key[i]) !=
> > > + ((u32 *)keys.enckey)[i]) {
> > > ctx->base.needs_inv = true;
> > > break;
> > > }
> > > @@ -459,7 +460,7 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, const u8
> > *key,
> > >
> > > /* Now copy the keys into the context */
> > > for (i = 0; i < keys.enckeylen / sizeof(u32); i++)
> > > - ctx->key[i] = cpu_to_le32(aes.key_enc[i]);
> > > + ctx->key[i] = cpu_to_le32(((u32 *)keys.enckey)[i]);
> > > ctx->key_len = keys.enckeylen;
> > >
> > > memcpy(ctx->ipad, &istate.state, ctx->state_sz);
> > > --
> > > 1.8.3.1
> > >
>
>
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> http://www.insidesecure.com