Re: Bug in geode-aes.c ?
On Thu, Nov 12, 2009 at 01:18:53PM +0300, Sergey Mironov wrote: > > Here is similar patch for aes_s390: It doesn't apply either: $ git apply ~/p patch: malformed patch at line 75: *tfm, const u8 *in_key, $ Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} 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
Re: Bug in geode-aes.c ?
On Thu, Nov 12, 2009 at 08:36:42AM +, Sergey Mironov wrote: > > Hi. Ok, here is geode-patch Your patch is corrupted: $ git apply ~/p patch: malformed patch at line 51: *tfm, const u8 *key, $ Please resend. Thanks, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} 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
Re: Bug in geode-aes.c ?
>From 335b39e0c55a1dba13cda3e8222947f2cb4120ed Mon Sep 17 00:00:00 2001 >From: Sergey Mironov >Date: Thu, 12 Nov 2009 13:10:05 +0300 >Subject: [PATCH 2/2] aes_s390: access fallback.cip cipher fallback mode > >|The fallback code in cipher mode touch the union fallback.blk instead >|of fallback.cip. This is wrong because we use the cipher and not the >|blockcipher. This did not show any side effects yet because both types / >|structs contain the same element right now. > >Signed-off-by: Sergey Mironov Looks good. >--- > arch/s390/crypto/aes_s390.c |8 > 1 files changed, 4 insertions(+), 4 deletions(-) > >diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c >index e33f32b..6f0f8b9 100644 >--- a/arch/s390/crypto/aes_s390.c >+++ b/arch/s390/crypto/aes_s390.c >@@ -75,14 +75,14 @@ static int setkey_fallback_cip(struct crypto_tfm >*tfm, const u8 *in_key, > struct s390_aes_ctx *sctx = crypto_tfm_ctx(tfm); > int ret; > >- sctx->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; >- sctx->fallback.blk->base.crt_flags |= (tfm->crt_flags & >+ sctx->fallback.cip->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; >+ sctx->fallback.cip->base.crt_flags |= (tfm->crt_flags & > CRYPTO_TFM_REQ_MASK); > > ret = crypto_cipher_setkey(sctx->fallback.cip, in_key, key_len); > if (ret) { > tfm->crt_flags &= ~CRYPTO_TFM_RES_MASK; >- tfm->crt_flags |= (sctx->fallback.blk->base.crt_flags & >+ tfm->crt_flags |= (sctx->fallback.cip->base.crt_flags & > CRYPTO_TFM_RES_MASK); > } > return ret; >@@ -170,7 +170,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm) > > if (IS_ERR(sctx->fallback.cip)) { > printk(KERN_ERR "Error allocating fallback algo %s\n", name); >- return PTR_ERR(sctx->fallback.blk); >+ return PTR_ERR(sctx->fallback.cip); > } > > return 0; >-- >1.6.4.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 -- 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: Bug in geode-aes.c ?
2009/11/12 Sebastian Andrzej Siewior : > >From 0a61b446585324a3041ef0a138515ef936a14eb7 Mon Sep 17 00:00:00 2001 >>From: Sergey Mironov >>Date: Thu, 12 Nov 2009 11:30:02 +0300 >>Subject: [PATCH] Fixed typo bugs in geod-aes.c > > On a second look could you please add something like: > > crypto/geode: access fallback.cip cipher fallback mode > > |The fallback code in cipher mode touch the union fallback.blk instead > |of fallback.cip. This is wrong because we use the cipher and not the > |blockcipher. This did not show any side effects yet because both types / > |structs contain the same element right now. > > >>Signed-off-by: Sergey Mironov >>--- >> drivers/crypto/geode-aes.c | 6 +++--- >> 1 files changed, 3 insertions(+), 3 deletions(-) >> >>diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c >>index 4801162..03e71b1 100644 >>--- a/drivers/crypto/geode-aes.c >>+++ b/drivers/crypto/geode-aes.c >>@@ -135,8 +135,8 @@ static int geode_setkey_cip(struct crypto_tfm >>*tfm, const u8 *key, >> /* >> * The requested key size is not supported by HW, do a fallback >> */ >>- op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; >>- op->fallback.blk->base.crt_flags |= (tfm->crt_flags & >>CRYPTO_TFM_REQ_MASK); >>+ op->fallback.cip->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; >>+ op->fallback.cip->base.crt_flags |= (tfm->crt_flags & >>CRYPTO_TFM_REQ_MASK); >> >> ret = crypto_cipher_setkey(op->fallback.cip, key, len); >> if (ret) { >>@@ -263,7 +263,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm) >> >> if (IS_ERR(op->fallback.cip)) { >> printk(KERN_ERR "Error allocating fallback algo %s\n", name); >>- return PTR_ERR(op->fallback.blk); >>+ return PTR_ERR(op->fallback.cip); >> } >> >> return 0; > > Sebastian > Here is similar patch for aes_s390: -- Sergey >From 335b39e0c55a1dba13cda3e8222947f2cb4120ed Mon Sep 17 00:00:00 2001 From: Sergey Mironov Date: Thu, 12 Nov 2009 13:10:05 +0300 Subject: [PATCH 2/2] aes_s390: access fallback.cip cipher fallback mode |The fallback code in cipher mode touch the union fallback.blk instead |of fallback.cip. This is wrong because we use the cipher and not the |blockcipher. This did not show any side effects yet because both types / |structs contain the same element right now. Signed-off-by: Sergey Mironov --- arch/s390/crypto/aes_s390.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c index e33f32b..6f0f8b9 100644 --- a/arch/s390/crypto/aes_s390.c +++ b/arch/s390/crypto/aes_s390.c @@ -75,14 +75,14 @@ static int setkey_fallback_cip(struct crypto_tfm *tfm, const u8 *in_key, struct s390_aes_ctx *sctx = crypto_tfm_ctx(tfm); int ret; - sctx->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; - sctx->fallback.blk->base.crt_flags |= (tfm->crt_flags & + sctx->fallback.cip->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; + sctx->fallback.cip->base.crt_flags |= (tfm->crt_flags & CRYPTO_TFM_REQ_MASK); ret = crypto_cipher_setkey(sctx->fallback.cip, in_key, key_len); if (ret) { tfm->crt_flags &= ~CRYPTO_TFM_RES_MASK; - tfm->crt_flags |= (sctx->fallback.blk->base.crt_flags & + tfm->crt_flags |= (sctx->fallback.cip->base.crt_flags & CRYPTO_TFM_RES_MASK); } return ret; @@ -170,7 +170,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm) if (IS_ERR(sctx->fallback.cip)) { printk(KERN_ERR "Error allocating fallback algo %s\n", name); - return PTR_ERR(sctx->fallback.blk); + return PTR_ERR(sctx->fallback.cip); } return 0; -- 1.6.4.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: Bug in geode-aes.c ?
>From 0a61b446585324a3041ef0a138515ef936a14eb7 Mon Sep 17 00:00:00 2001 >From: Sergey Mironov >Date: Thu, 12 Nov 2009 11:30:02 +0300 >Subject: [PATCH] Fixed typo bugs in geod-aes.c On a second look could you please add something like: crypto/geode: access fallback.cip cipher fallback mode |The fallback code in cipher mode touch the union fallback.blk instead |of fallback.cip. This is wrong because we use the cipher and not the |blockcipher. This did not show any side effects yet because both types / |structs contain the same element right now. >Signed-off-by: Sergey Mironov >--- > drivers/crypto/geode-aes.c |6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > >diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c >index 4801162..03e71b1 100644 >--- a/drivers/crypto/geode-aes.c >+++ b/drivers/crypto/geode-aes.c >@@ -135,8 +135,8 @@ static int geode_setkey_cip(struct crypto_tfm >*tfm, const u8 *key, > /* >* The requested key size is not supported by HW, do a fallback >*/ >- op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; >- op->fallback.blk->base.crt_flags |= (tfm->crt_flags & >CRYPTO_TFM_REQ_MASK); >+ op->fallback.cip->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; >+ op->fallback.cip->base.crt_flags |= (tfm->crt_flags & >CRYPTO_TFM_REQ_MASK); > > ret = crypto_cipher_setkey(op->fallback.cip, key, len); > if (ret) { >@@ -263,7 +263,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm) > > if (IS_ERR(op->fallback.cip)) { > printk(KERN_ERR "Error allocating fallback algo %s\n", name); >- return PTR_ERR(op->fallback.blk); >+ return PTR_ERR(op->fallback.cip); > } > > return 0; Sebastian -- 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: Bug in geode-aes.c ?
* Sergey Mironov | 2009-11-12 11:43:20 [+0300]: >I can't find any "s390" reference in my tree. Probably, i have not got >this branch in my local copy. If you want to fix this please look at arch/s390/crypto/aes_s390.c. That is broken and I mixed it up in fallback_init_cip() as well :) Sebastian -- 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: Bug in geode-aes.c ?
* Sergey Mironov | 2009-11-12 11:36:42 [+0300]: >From 0a61b446585324a3041ef0a138515ef936a14eb7 Mon Sep 17 00:00:00 2001 >From: Sergey Mironov >Date: Thu, 12 Nov 2009 11:30:02 +0300 >Subject: [PATCH] Fixed typo bugs in geod-aes.c > > >Signed-off-by: Sergey Mironov Acked-by: Sebastian Andrzej Siewior I have the feeling that the driver will deadlock anyway. Got to check this once I find the board.. >--- > drivers/crypto/geode-aes.c |6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > >diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c >index 4801162..03e71b1 100644 >--- a/drivers/crypto/geode-aes.c >+++ b/drivers/crypto/geode-aes.c >@@ -135,8 +135,8 @@ static int geode_setkey_cip(struct crypto_tfm >*tfm, const u8 *key, > /* >* The requested key size is not supported by HW, do a fallback >*/ >- op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; >- op->fallback.blk->base.crt_flags |= (tfm->crt_flags & >CRYPTO_TFM_REQ_MASK); >+ op->fallback.cip->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; >+ op->fallback.cip->base.crt_flags |= (tfm->crt_flags & >CRYPTO_TFM_REQ_MASK); > > ret = crypto_cipher_setkey(op->fallback.cip, key, len); > if (ret) { >@@ -263,7 +263,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm) > > if (IS_ERR(op->fallback.cip)) { > printk(KERN_ERR "Error allocating fallback algo %s\n", name); >- return PTR_ERR(op->fallback.blk); >+ return PTR_ERR(op->fallback.cip); > } > > return 0; >-- >1.6.4.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: Bug in geode-aes.c ?
2009/11/12 Sergey Mironov : > 2009/11/12 Sebastian Andrzej Siewior : >> * Sergey Mironov | 2009-11-10 17:00:31 [+0300]: >> >>> 116 static int geode_setkey_cip(struct crypto_tfm *tfm, const u8 *key, >>> 117 unsigned int len) >>> 118 { >>>... >>> >>>/** BUG? Should it be 'op->fallback.cip' instead of 'op->fallback.blk' ? **/ >>> >>> 138 op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; >>> 139 op->fallback.blk->base.crt_flags |= (tfm->crt_flags & >>>CRYPTO_TFM_REQ_MASK); >>> >>>... >>> >>> 144 tfm->crt_flags |= >>>(op->fallback.blk->base.crt_flags & CRYPTO_TFM_RES_MASK); >>> 145 } >>> 146 return ret; >> >> Yup, good catch. It has to be cip instead of blk. I've copy/pasted it >> and the same bug is in s390's crypto driver. No one noticed it because >> both structs are equal, just the name / type is different. >> Do you mind sending a patch? >> >> Sebastian >> > > Hi. Ok, here is geode-patch > -- > Sergey > > > From 0a61b446585324a3041ef0a138515ef936a14eb7 Mon Sep 17 00:00:00 2001 > From: Sergey Mironov > Date: Thu, 12 Nov 2009 11:30:02 +0300 > Subject: [PATCH] Fixed typo bugs in geod-aes.c > > > Signed-off-by: Sergey Mironov > --- > drivers/crypto/geode-aes.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c > index 4801162..03e71b1 100644 > --- a/drivers/crypto/geode-aes.c > +++ b/drivers/crypto/geode-aes.c > @@ -135,8 +135,8 @@ static int geode_setkey_cip(struct crypto_tfm > *tfm, const u8 *key, > /* > * The requested key size is not supported by HW, do a fallback > */ > - op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; > - op->fallback.blk->base.crt_flags |= (tfm->crt_flags & > CRYPTO_TFM_REQ_MASK); > + op->fallback.cip->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; > + op->fallback.cip->base.crt_flags |= (tfm->crt_flags & > CRYPTO_TFM_REQ_MASK); > > ret = crypto_cipher_setkey(op->fallback.cip, key, len); > if (ret) { > @@ -263,7 +263,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm) > > if (IS_ERR(op->fallback.cip)) { > printk(KERN_ERR "Error allocating fallback algo %s\n", name); > - return PTR_ERR(op->fallback.blk); > + return PTR_ERR(op->fallback.cip); > } > > return 0; > -- > 1.6.4.4 > I can't find any "s390" reference in my tree. Probably, i have not got this branch in my local copy. -- Sergey -- 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: Bug in geode-aes.c ?
2009/11/12 Sebastian Andrzej Siewior : > * Sergey Mironov | 2009-11-10 17:00:31 [+0300]: > >> 116 static int geode_setkey_cip(struct crypto_tfm *tfm, const u8 *key, >> 117 unsigned int len) >> 118 { >>... >> >>/** BUG? Should it be 'op->fallback.cip' instead of 'op->fallback.blk' ? **/ >> >> 138 op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; >> 139 op->fallback.blk->base.crt_flags |= (tfm->crt_flags & >>CRYPTO_TFM_REQ_MASK); >> >>... >> >> 144 tfm->crt_flags |= >>(op->fallback.blk->base.crt_flags & CRYPTO_TFM_RES_MASK); >> 145 } >> 146 return ret; > > Yup, good catch. It has to be cip instead of blk. I've copy/pasted it > and the same bug is in s390's crypto driver. No one noticed it because > both structs are equal, just the name / type is different. > Do you mind sending a patch? > > Sebastian > Hi. Ok, here is geode-patch -- Sergey >From 0a61b446585324a3041ef0a138515ef936a14eb7 Mon Sep 17 00:00:00 2001 From: Sergey Mironov Date: Thu, 12 Nov 2009 11:30:02 +0300 Subject: [PATCH] Fixed typo bugs in geod-aes.c Signed-off-by: Sergey Mironov --- drivers/crypto/geode-aes.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c index 4801162..03e71b1 100644 --- a/drivers/crypto/geode-aes.c +++ b/drivers/crypto/geode-aes.c @@ -135,8 +135,8 @@ static int geode_setkey_cip(struct crypto_tfm *tfm, const u8 *key, /* * The requested key size is not supported by HW, do a fallback */ - op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; - op->fallback.blk->base.crt_flags |= (tfm->crt_flags & CRYPTO_TFM_REQ_MASK); + op->fallback.cip->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; + op->fallback.cip->base.crt_flags |= (tfm->crt_flags & CRYPTO_TFM_REQ_MASK); ret = crypto_cipher_setkey(op->fallback.cip, key, len); if (ret) { @@ -263,7 +263,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm) if (IS_ERR(op->fallback.cip)) { printk(KERN_ERR "Error allocating fallback algo %s\n", name); - return PTR_ERR(op->fallback.blk); + return PTR_ERR(op->fallback.cip); } return 0; -- 1.6.4.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: Bug in geode-aes.c ?
* Sergey Mironov | 2009-11-10 17:00:31 [+0300]: > 116 static int geode_setkey_cip(struct crypto_tfm *tfm, const u8 *key, > 117 unsigned int len) > 118 { >... > >/** BUG? Should it be 'op->fallback.cip' instead of 'op->fallback.blk' ? **/ > > 138 op->fallback.blk->base.crt_flags &= ~CRYPTO_TFM_REQ_MASK; > 139 op->fallback.blk->base.crt_flags |= (tfm->crt_flags & >CRYPTO_TFM_REQ_MASK); > >... > > 144 tfm->crt_flags |= >(op->fallback.blk->base.crt_flags & CRYPTO_TFM_RES_MASK); > 145 } > 146 return ret; Yup, good catch. It has to be cip instead of blk. I've copy/pasted it and the same bug is in s390's crypto driver. No one noticed it because both structs are equal, just the name / type is different. Do you mind sending a patch? Sebastian -- 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