Hello. I noticed strange thing in drivers/crypto/geode-aes.c (
function geode_setkey_cip() )
This code still the same in git://linux-arm.org/linux-2.6-stable.git
(tag v2.6.28-arm2)
Please, check.
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;
--
Thanks,
Sergey
* 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
2009/11/12 Sebastian Andrzej Siewior <[email protected]>:
> * 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 <[email protected]>
Date: Thu, 12 Nov 2009 11:30:02 +0300
Subject: [PATCH] Fixed typo bugs in geod-aes.c
Signed-off-by: Sergey Mironov <[email protected]>
---
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
2009/11/12 Sergey Mironov <[email protected]>:
> 2009/11/12 Sebastian Andrzej Siewior <[email protected]>:
>> * 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 <[email protected]>
> Date: Thu, 12 Nov 2009 11:30:02 +0300
> Subject: [PATCH] Fixed typo bugs in geod-aes.c
>
>
> Signed-off-by: Sergey Mironov <[email protected]>
> ---
> ?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
* Sergey Mironov | 2009-11-12 11:36:42 [+0300]:
>From 0a61b446585324a3041ef0a138515ef936a14eb7 Mon Sep 17 00:00:00 2001
>From: Sergey Mironov <[email protected]>
>Date: Thu, 12 Nov 2009 11:30:02 +0300
>Subject: [PATCH] Fixed typo bugs in geod-aes.c
>
>
>Signed-off-by: Sergey Mironov <[email protected]>
Acked-by: Sebastian Andrzej Siewior <[email protected]>
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
* 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
>From 0a61b446585324a3041ef0a138515ef936a14eb7 Mon Sep 17 00:00:00 2001
>From: Sergey Mironov <[email protected]>
>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 <[email protected]>
>---
> 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
2009/11/12 Sebastian Andrzej Siewior <[email protected]>:
> >From 0a61b446585324a3041ef0a138515ef936a14eb7 Mon Sep 17 00:00:00 2001
>>From: Sergey Mironov <[email protected]>
>>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 <[email protected]>
>>---
>> 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 <[email protected]>
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 <[email protected]>
---
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
>From 335b39e0c55a1dba13cda3e8222947f2cb4120ed Mon Sep 17 00:00:00 2001
>From: Sergey Mironov <[email protected]>
>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 <[email protected]>
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 [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 12, 2009 at 08:36:42AM +0000, 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~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
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~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt