* [PATCH] crypto: cmac - fix alignment of 'consts'
@ 2016-10-10 17:15 Eric Biggers
2016-10-10 17:29 ` Joe Perches
2016-10-21 3:15 ` Herbert Xu
0 siblings, 2 replies; 6+ messages in thread
From: Eric Biggers @ 2016-10-10 17:15 UTC (permalink / raw)
To: herbert, davem; +Cc: linux-crypto, linux-kernel, Eric Biggers
The per-transform 'consts' array is accessed as __be64 in
crypto_cmac_digest_setkey() but was only guaranteed to be aligned to
__alignof__(long). Fix this by aligning it to __alignof__(__be64).
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/cmac.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/crypto/cmac.c b/crypto/cmac.c
index b6c4059..04080dc 100644
--- a/crypto/cmac.c
+++ b/crypto/cmac.c
@@ -57,7 +57,8 @@ static int crypto_cmac_digest_setkey(struct crypto_shash *parent,
unsigned long alignmask = crypto_shash_alignmask(parent);
struct cmac_tfm_ctx *ctx = crypto_shash_ctx(parent);
unsigned int bs = crypto_shash_blocksize(parent);
- __be64 *consts = PTR_ALIGN((void *)ctx->ctx, alignmask + 1);
+ __be64 *consts = PTR_ALIGN((void *)ctx->ctx,
+ (alignmask | (__alignof__(__be64) - 1)) + 1);
u64 _const[2];
int i, err = 0;
u8 msb_mask, gfmask;
@@ -173,7 +174,8 @@ static int crypto_cmac_digest_final(struct shash_desc *pdesc, u8 *out)
struct cmac_desc_ctx *ctx = shash_desc_ctx(pdesc);
struct crypto_cipher *tfm = tctx->child;
int bs = crypto_shash_blocksize(parent);
- u8 *consts = PTR_ALIGN((void *)tctx->ctx, alignmask + 1);
+ u8 *consts = PTR_ALIGN((void *)tctx->ctx,
+ (alignmask | (__alignof__(__be64) - 1)) + 1);
u8 *odds = PTR_ALIGN((void *)ctx->ctx, alignmask + 1);
u8 *prev = odds + bs;
unsigned int offset = 0;
@@ -258,7 +260,8 @@ static int cmac_create(struct crypto_template *tmpl, struct rtattr **tb)
if (err)
goto out_free_inst;
- alignmask = alg->cra_alignmask | (sizeof(long) - 1);
+ /* We access the data as u32s when xoring. */
+ alignmask = alg->cra_alignmask | (__alignof__(u32) - 1);
inst->alg.base.cra_alignmask = alignmask;
inst->alg.base.cra_priority = alg->cra_priority;
inst->alg.base.cra_blocksize = alg->cra_blocksize;
@@ -270,7 +273,9 @@ static int cmac_create(struct crypto_template *tmpl, struct rtattr **tb)
+ alg->cra_blocksize * 2;
inst->alg.base.cra_ctxsize =
- ALIGN(sizeof(struct cmac_tfm_ctx), alignmask + 1)
+ ALIGN(sizeof(struct cmac_tfm_ctx), crypto_tfm_ctx_alignment())
+ + ((alignmask | (__alignof__(__be64) - 1)) &
+ ~(crypto_tfm_ctx_alignment() - 1))
+ alg->cra_blocksize * 2;
inst->alg.base.cra_init = cmac_init_tfm;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: cmac - fix alignment of 'consts'
2016-10-10 17:15 [PATCH] crypto: cmac - fix alignment of 'consts' Eric Biggers
@ 2016-10-10 17:29 ` Joe Perches
2016-10-10 17:37 ` Eric Biggers
2016-10-21 3:15 ` Herbert Xu
1 sibling, 1 reply; 6+ messages in thread
From: Joe Perches @ 2016-10-10 17:29 UTC (permalink / raw)
To: Eric Biggers, herbert, davem; +Cc: linux-crypto, linux-kernel
On Mon, 2016-10-10 at 10:15 -0700, Eric Biggers wrote:
> The per-transform 'consts' array is accessed as __be64 in
> crypto_cmac_digest_setkey() but was only guaranteed to be aligned to
> __alignof__(long). Fix this by aligning it to __alignof__(__be64).
[]
> diff --git a/crypto/cmac.c b/crypto/cmac.c
[]
> @@ -57,7 +57,8 @@ static int crypto_cmac_digest_setkey(struct crypto_shash *parent,
> unsigned long alignmask = crypto_shash_alignmask(parent);
> struct cmac_tfm_ctx *ctx = crypto_shash_ctx(parent);
> unsigned int bs = crypto_shash_blocksize(parent);
> - __be64 *consts = PTR_ALIGN((void *)ctx->ctx, alignmask + 1);
> + __be64 *consts = PTR_ALIGN((void *)ctx->ctx,
> + (alignmask | (__alignof__(__be64) - 1)) + 1);
Using a bitwise or looks very odd there. Perhaps:
min(alignmask + 1, __alignof__(__be64))
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: cmac - fix alignment of 'consts'
2016-10-10 17:29 ` Joe Perches
@ 2016-10-10 17:37 ` Eric Biggers
2016-10-10 17:51 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Eric Biggers @ 2016-10-10 17:37 UTC (permalink / raw)
To: Joe Perches; +Cc: herbert, davem, linux-crypto, linux-kernel
On Mon, Oct 10, 2016 at 10:29:55AM -0700, Joe Perches wrote:
> On Mon, 2016-10-10 at 10:15 -0700, Eric Biggers wrote:
> > The per-transform 'consts' array is accessed as __be64 in
> > crypto_cmac_digest_setkey() but was only guaranteed to be aligned to
> > __alignof__(long). Fix this by aligning it to __alignof__(__be64).
> []
> > diff --git a/crypto/cmac.c b/crypto/cmac.c
> []
> > @@ -57,7 +57,8 @@ static int crypto_cmac_digest_setkey(struct crypto_shash *parent,
> > unsigned long alignmask = crypto_shash_alignmask(parent);
> > struct cmac_tfm_ctx *ctx = crypto_shash_ctx(parent);
> > unsigned int bs = crypto_shash_blocksize(parent);
> > - __be64 *consts = PTR_ALIGN((void *)ctx->ctx, alignmask + 1);
> > + __be64 *consts = PTR_ALIGN((void *)ctx->ctx,
> > + (alignmask | (__alignof__(__be64) - 1)) + 1);
>
> Using a bitwise or looks very odd there. Perhaps:
>
> min(alignmask + 1, __alignof__(__be64))
>
Alignment has to be a power of 2. From the code I've read, crypto drivers work
with alignment a lot and use bitwise OR to mean "the more restrictive of these
alignmasks". So I believe the way it's written is the preferred style.
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: cmac - fix alignment of 'consts'
2016-10-10 17:37 ` Eric Biggers
@ 2016-10-10 17:51 ` Joe Perches
2016-10-10 18:07 ` Eric Biggers
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2016-10-10 17:51 UTC (permalink / raw)
To: Eric Biggers; +Cc: herbert, davem, linux-crypto, linux-kernel
On Mon, 2016-10-10 at 10:37 -0700, Eric Biggers wrote:
> On Mon, Oct 10, 2016 at 10:29:55AM -0700, Joe Perches wrote:
> > On Mon, 2016-10-10 at 10:15 -0700, Eric Biggers wrote:
> > > The per-transform 'consts' array is accessed as __be64 in
> > > crypto_cmac_digest_setkey() but was only guaranteed to be aligned to
> > > __alignof__(long). Fix this by aligning it to __alignof__(__be64).
> > []
> > > diff --git a/crypto/cmac.c b/crypto/cmac.c
> > []
> > > @@ -57,7 +57,8 @@ static int crypto_cmac_digest_setkey(struct crypto_shash *parent,
> > > unsigned long alignmask = crypto_shash_alignmask(parent);
> > > struct cmac_tfm_ctx *ctx = crypto_shash_ctx(parent);
> > > unsigned int bs = crypto_shash_blocksize(parent);
> > > - __be64 *consts = PTR_ALIGN((void *)ctx->ctx, alignmask + 1);
> > > + __be64 *consts = PTR_ALIGN((void *)ctx->ctx,
> > > + (alignmask | (__alignof__(__be64) - 1)) + 1);
> >
> >
> > Using a bitwise or looks very odd there. Perhaps:
> >
> > min(alignmask + 1, __alignof__(__be64))
> >
>
>
> Alignment has to be a power of 2. From the code I've read, crypto drivers work
> with alignment a lot and use bitwise OR to mean "the more restrictive of these
> alignmasks". So I believe the way it's written is the preferred style.
>
> Eric
Hey Eric.
I don't see any PTR_ALIGN uses in crypto/ or drivers/crypto/ that
use a bitwise or, just mask + 1, but I believe the effect is the
same. Anyway, your choice, but I think using min is clearer.
cheers, Joe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: cmac - fix alignment of 'consts'
2016-10-10 17:51 ` Joe Perches
@ 2016-10-10 18:07 ` Eric Biggers
0 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2016-10-10 18:07 UTC (permalink / raw)
To: Joe Perches; +Cc: herbert, davem, linux-crypto, linux-kernel
On Mon, Oct 10, 2016 at 10:51:14AM -0700, Joe Perches wrote:
>
> Hey Eric.
>
> I don't see any PTR_ALIGN uses in crypto/ or drivers/crypto/ that
> use a bitwise or, just mask + 1, but I believe the effect is the
> same. Anyway, your choice, but I think using min is clearer.
>
> cheers, Joe
Usually the bitwise OR is used when setting cra_alignmask in the 'struct
crypto_alg'. Indeed, the problem could be solved by setting
inst->alg.base.cra_alignmask = alg->cra_alignmask | (__alignof__(__be64) - 1);
I decided against that because it would always force 8-byte alignment for CMAC
input buffers and keys, when in fact they don't need that level of alignment
unless the underlying block cipher requires it.
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: cmac - fix alignment of 'consts'
2016-10-10 17:15 [PATCH] crypto: cmac - fix alignment of 'consts' Eric Biggers
2016-10-10 17:29 ` Joe Perches
@ 2016-10-21 3:15 ` Herbert Xu
1 sibling, 0 replies; 6+ messages in thread
From: Herbert Xu @ 2016-10-21 3:15 UTC (permalink / raw)
To: Eric Biggers; +Cc: davem, linux-crypto, linux-kernel
On Mon, Oct 10, 2016 at 10:15:15AM -0700, Eric Biggers wrote:
> The per-transform 'consts' array is accessed as __be64 in
> crypto_cmac_digest_setkey() but was only guaranteed to be aligned to
> __alignof__(long). Fix this by aligning it to __alignof__(__be64).
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-21 3:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-10 17:15 [PATCH] crypto: cmac - fix alignment of 'consts' Eric Biggers
2016-10-10 17:29 ` Joe Perches
2016-10-10 17:37 ` Eric Biggers
2016-10-10 17:51 ` Joe Perches
2016-10-10 18:07 ` Eric Biggers
2016-10-21 3:15 ` Herbert Xu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.