Hello Herbert,
about the aligment in the crypto API, I'm lost once again.
In commit f10b7897ee29649fa7f0ccdc8d859ccd6ce7dbfd you extended the
alignment to be as wide as possible (i.e. algo specified 3, natural is
8, do 8).
Currently crypto_ctxsize(), what is used by __crypto_alloc_tfm() in
order to allocate space for tfm + priv ctx, looks like:
len = alg->cra_alignmask & ~(crypto_tfm_ctx_alignment() - 1);
if (type_obj)
return len + type_obj->ctxsize(alg, type, mask);
Now, in my case alg->cra_alignmask is 15 and crypto_tfm_ctx_alignment()
returns 8 (ppc64). This makes 15 & ~(8-1) = 8 or too less. My mistake,
make it 16 instead of 15 and everything is fine again. aes.c + cbc.c
specify a mask of 3. The same thing once again: 3 & ~(8-1) = 0. This
makes me thing, that the line that calculates len, should maybe be
len = alg->cra_alignmask | (crypto_tfm_ctx_alignment() - 1);
or?
I noticed this, after my code did not work properly. The reason was,
that my private ctx was not retrieved with
crypto_ablkcipher_ctx_aligned() (attached) but with
crypto_ablkcipher_ctx() (and it was not properly aligned anymore).
My fault right?
Shouldn't the behavior of crypto_blkcipher_ctx_aligned() (which is
currently unused) be the default one?
Sebastian
---
--- a/include/crypto/algapi.h
+++ b/include/crypto/algapi.h
@@ -160,6 +160,11 @@ static inline void *crypto_ablkcipher_ct
return crypto_tfm_ctx(&tfm->base);
}
+static inline void *crypto_ablkcipher_ctx_aligned(struct crypto_ablkcipher *tfm)
+{
+ return crypto_tfm_ctx_aligned(&tfm->base);
+}
+
static inline struct crypto_blkcipher *crypto_spawn_blkcipher(
struct crypto_spawn *spawn)
{
On Thu, Aug 02, 2007 at 01:02:40AM +0200, Sebastian Siewior wrote:
>
> I noticed this, after my code did not work properly. The reason was,
> that my private ctx was not retrieved with
> crypto_ablkcipher_ctx_aligned() (attached) but with
> crypto_ablkcipher_ctx() (and it was not properly aligned anymore).
> My fault right?
Yeah if your ctx needs to be aligned you need to use
crypto_tfm_ctx_aligned.
> Shouldn't the behavior of crypto_blkcipher_ctx_aligned() (which is
> currently unused) be the default one?
Most of our existing algorithms don't care about alignment.
> --- a/include/crypto/algapi.h
> +++ b/include/crypto/algapi.h
> @@ -160,6 +160,11 @@ static inline void *crypto_ablkcipher_ct
> return crypto_tfm_ctx(&tfm->base);
> }
>
> +static inline void *crypto_ablkcipher_ctx_aligned(struct crypto_ablkcipher *tfm)
> +{
> + return crypto_tfm_ctx_aligned(&tfm->base);
> +}
> +
> static inline struct crypto_blkcipher *crypto_spawn_blkcipher(
> struct crypto_spawn *spawn)
Looks good. Could you add a description and sign-off?
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
* Herbert Xu | 2007-08-02 10:30:06 [+0800]:
>> I noticed this, after my code did not work properly. The reason was,
>> that my private ctx was not retrieved with
>> crypto_ablkcipher_ctx_aligned() (attached) but with
>> crypto_ablkcipher_ctx() (and it was not properly aligned anymore).
>> My fault right?
>
>Yeah if your ctx needs to be aligned you need to use
>crypto_tfm_ctx_aligned.
Okey this makes sense. What about crypto_ctxsize()? Isn't a bug there?
I can't specify an alignmask of 16 (what I assumed yesterday after a
part of brain switched to sleep()) because this brakes the ALIGN()
macro. Isn't something like
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -273,7 +273,7 @@ static unsigned int crypto_ctxsize(struc
const struct crypto_type *type_obj = alg->cra_type;
unsigned int len;
- len = alg->cra_alignmask & ~(crypto_tfm_ctx_alignment() - 1);
+ len = alg->cra_alignmask | (crypto_tfm_ctx_alignment() -1)
if (type_obj)
return len + type_obj->ctxsize(alg, type, mask);
----
required? VIA's PadLock driver uses manully ALIGN() instead of
crypto_.*_ctx_aligned() to do this. Therefore it will overwrite memory
or am I overlooking something?
>> --- a/include/crypto/algapi.h
>> +++ b/include/crypto/algapi.h
>> @@ -160,6 +160,11 @@ static inline void *crypto_ablkcipher_ct
>> return crypto_tfm_ctx(&tfm->base);
>> }
>>
>> +static inline void *crypto_ablkcipher_ctx_aligned(struct crypto_ablkcipher *tfm)
>> +{
>> + return crypto_tfm_ctx_aligned(&tfm->base);
>> +}
>> +
>> static inline struct crypto_blkcipher *crypto_spawn_blkcipher(
>> struct crypto_spawn *spawn)
>
>Looks good. Could you add a description and sign-off?
Yes, no problem.
Sebastian
On Thu, Aug 02, 2007 at 09:41:22AM +0200, Sebastian Siewior wrote:
>
> Okey this makes sense. What about crypto_ctxsize()? Isn't a bug there?
> I can't specify an alignmask of 16 (what I assumed yesterday after a
> part of brain switched to sleep()) because this brakes the ALIGN()
> macro. Isn't something like
15 & ~7 == 8
is sufficient to guarantee 16-byte alignment. Remember that
the ctx member is already 8-byte aligned. So we only need
to add a maximum of 8 extra bytes to get it 16-byte aligned.
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