| Message ID | 20190624073818.29296-2-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: aegis128 - add NEON intrinsics version for ARM/arm64 | expand |
Hi Ard, On Mon, Jun 24, 2019 at 9:38 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > Use crypto_aegis128_update_u() not crypto_aegis128_update_a() in the > decrypt path that is taken when the source or destination pointers > are not aligned. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > crypto/aegis128.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/crypto/aegis128.c b/crypto/aegis128.c > index d78f77fc5dd1..125e11246990 100644 > --- a/crypto/aegis128.c > +++ b/crypto/aegis128.c > @@ -208,7 +208,7 @@ static void crypto_aegis128_decrypt_chunk(struct aegis_state *state, u8 *dst, > crypto_aegis_block_xor(&tmp, &state->blocks[1]); > crypto_xor(tmp.bytes, src, AEGIS_BLOCK_SIZE); > > - crypto_aegis128_update_a(state, &tmp); > + crypto_aegis128_update_u(state, &tmp); The "tmp" variable used here is declared directly on the stack as 'union aegis_block' and thus should be aligned to alignof(__le64), which allows the use of crypto_aegis128_update_a() -> crypto_aegis_block_xor(). It is also passed directly to crypto_aegis_block_xor() a few lines above. Or am I missing something? > > memcpy(dst, tmp.bytes, AEGIS_BLOCK_SIZE); > > -- > 2.20.1 > -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
On Mon, 24 Jun 2019 at 09:59, Ondrej Mosnacek <omosnace@redhat.com> wrote: > > Hi Ard, > > On Mon, Jun 24, 2019 at 9:38 AM Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: > > > > Use crypto_aegis128_update_u() not crypto_aegis128_update_a() in the > > decrypt path that is taken when the source or destination pointers > > are not aligned. > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > crypto/aegis128.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/crypto/aegis128.c b/crypto/aegis128.c > > index d78f77fc5dd1..125e11246990 100644 > > --- a/crypto/aegis128.c > > +++ b/crypto/aegis128.c > > @@ -208,7 +208,7 @@ static void crypto_aegis128_decrypt_chunk(struct aegis_state *state, u8 *dst, > > crypto_aegis_block_xor(&tmp, &state->blocks[1]); > > crypto_xor(tmp.bytes, src, AEGIS_BLOCK_SIZE); > > > > - crypto_aegis128_update_a(state, &tmp); > > + crypto_aegis128_update_u(state, &tmp); > > The "tmp" variable used here is declared directly on the stack as > 'union aegis_block' and thus should be aligned to alignof(__le64), > which allows the use of crypto_aegis128_update_a() -> > crypto_aegis_block_xor(). It is also passed directly to > crypto_aegis_block_xor() a few lines above. Or am I missing something? > Ah yes, you are absolutely right. Apologies for the noise. I just noticed the asymmetry with the encrypt path, but I should have looked more carefully. Please disregard this patch.
diff --git a/crypto/aegis128.c b/crypto/aegis128.c index d78f77fc5dd1..125e11246990 100644 --- a/crypto/aegis128.c +++ b/crypto/aegis128.c @@ -208,7 +208,7 @@ static void crypto_aegis128_decrypt_chunk(struct aegis_state *state, u8 *dst, crypto_aegis_block_xor(&tmp, &state->blocks[1]); crypto_xor(tmp.bytes, src, AEGIS_BLOCK_SIZE); - crypto_aegis128_update_a(state, &tmp); + crypto_aegis128_update_u(state, &tmp); memcpy(dst, tmp.bytes, AEGIS_BLOCK_SIZE);
Use crypto_aegis128_update_u() not crypto_aegis128_update_a() in the decrypt path that is taken when the source or destination pointers are not aligned. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- crypto/aegis128.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.20.1