diff mbox series

[1/2] crypto: morus/generic - fix for big endian systems

Message ID 20180930085859.15038-2-ard.biesheuvel@linaro.org
State Superseded
Headers show
Series crypto - fix aegis/morus for big endian systems | expand

Commit Message

Ard Biesheuvel Sept. 30, 2018, 8:58 a.m. UTC
Omit the endian swabbing when folding the lengths of the assoc and
crypt input buffers into the state to finalize the tag. This is not
necessary given that the memory representation of the state is in
machine native endianness already.

This fixes an error reported by tcrypt running on a big endian system:

  alg: aead: Test 2 failed on encryption for morus640-generic
  00000000: a8 30 ef fb e6 26 eb 23 b0 87 dd 98 57 f3 e1 4b
  00000010: 21
  alg: aead: Test 2 failed on encryption for morus1280-generic
  00000000: 88 19 1b fb 1c 29 49 0e ee 82 2f cb 97 a6 a5 ee
  00000010: 5f

Fixes: 396be41f16fd ("crypto: morus - Add generic MORUS AEAD implementations")
Cc: <stable@vger.kernel.org> # v4.18+
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 crypto/morus1280.c |  7 ++-----
 crypto/morus640.c  | 16 ++++------------
 2 files changed, 6 insertions(+), 17 deletions(-)

-- 
2.19.0

Comments

Ondrej Mosnacek Oct. 1, 2018, 7:26 a.m. UTC | #1
On Sun, Sep 30, 2018 at 10:59 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> Omit the endian swabbing when folding the lengths of the assoc and

> crypt input buffers into the state to finalize the tag. This is not

> necessary given that the memory representation of the state is in

> machine native endianness already.

>

> This fixes an error reported by tcrypt running on a big endian system:

>

>   alg: aead: Test 2 failed on encryption for morus640-generic

>   00000000: a8 30 ef fb e6 26 eb 23 b0 87 dd 98 57 f3 e1 4b

>   00000010: 21

>   alg: aead: Test 2 failed on encryption for morus1280-generic

>   00000000: 88 19 1b fb 1c 29 49 0e ee 82 2f cb 97 a6 a5 ee

>   00000010: 5f


Yikes, I never really got around to test MORUS and AEGIS on a BE
machine...  My mistake, sorry :/

>

> Fixes: 396be41f16fd ("crypto: morus - Add generic MORUS AEAD implementations")

> Cc: <stable@vger.kernel.org> # v4.18+

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>


> ---

>  crypto/morus1280.c |  7 ++-----

>  crypto/morus640.c  | 16 ++++------------

>  2 files changed, 6 insertions(+), 17 deletions(-)

>

> diff --git a/crypto/morus1280.c b/crypto/morus1280.c

> index d057cf5ac4a8..3889c188f266 100644

> --- a/crypto/morus1280.c

> +++ b/crypto/morus1280.c

> @@ -385,14 +385,11 @@ static void crypto_morus1280_final(struct morus1280_state *state,

>                                    struct morus1280_block *tag_xor,

>                                    u64 assoclen, u64 cryptlen)

>  {

> -       u64 assocbits = assoclen * 8;

> -       u64 cryptbits = cryptlen * 8;

> -

>         struct morus1280_block tmp;

>         unsigned int i;

>

> -       tmp.words[0] = cpu_to_le64(assocbits);

> -       tmp.words[1] = cpu_to_le64(cryptbits);

> +       tmp.words[0] = assoclen * 8;

> +       tmp.words[1] = cryptlen * 8;

>         tmp.words[2] = 0;

>         tmp.words[3] = 0;

>

> diff --git a/crypto/morus640.c b/crypto/morus640.c

> index 1ca76e54281b..da06ec2f6a80 100644

> --- a/crypto/morus640.c

> +++ b/crypto/morus640.c

> @@ -384,21 +384,13 @@ static void crypto_morus640_final(struct morus640_state *state,

>                                   struct morus640_block *tag_xor,

>                                   u64 assoclen, u64 cryptlen)

>  {

> -       u64 assocbits = assoclen * 8;

> -       u64 cryptbits = cryptlen * 8;

> -

> -       u32 assocbits_lo = (u32)assocbits;

> -       u32 assocbits_hi = (u32)(assocbits >> 32);

> -       u32 cryptbits_lo = (u32)cryptbits;

> -       u32 cryptbits_hi = (u32)(cryptbits >> 32);

> -

>         struct morus640_block tmp;

>         unsigned int i;

>

> -       tmp.words[0] = cpu_to_le32(assocbits_lo);

> -       tmp.words[1] = cpu_to_le32(assocbits_hi);

> -       tmp.words[2] = cpu_to_le32(cryptbits_lo);

> -       tmp.words[3] = cpu_to_le32(cryptbits_hi);

> +       tmp.words[0] = lower_32_bits(assoclen * 8);

> +       tmp.words[1] = upper_32_bits(assoclen * 8);

> +       tmp.words[2] = lower_32_bits(cryptlen * 8);

> +       tmp.words[3] = upper_32_bits(cryptlen * 8);

>

>         for (i = 0; i < MORUS_BLOCK_WORDS; i++)

>                 state->s[4].words[i] ^= state->s[0].words[i];

> --

> 2.19.0

>


Thanks,

--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
Ard Biesheuvel Oct. 1, 2018, 7:36 a.m. UTC | #2
On 1 October 2018 at 09:26, Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Sun, Sep 30, 2018 at 10:59 AM Ard Biesheuvel

> <ard.biesheuvel@linaro.org> wrote:

>> Omit the endian swabbing when folding the lengths of the assoc and

>> crypt input buffers into the state to finalize the tag. This is not

>> necessary given that the memory representation of the state is in

>> machine native endianness already.

>>

>> This fixes an error reported by tcrypt running on a big endian system:

>>

>>   alg: aead: Test 2 failed on encryption for morus640-generic

>>   00000000: a8 30 ef fb e6 26 eb 23 b0 87 dd 98 57 f3 e1 4b

>>   00000010: 21

>>   alg: aead: Test 2 failed on encryption for morus1280-generic

>>   00000000: 88 19 1b fb 1c 29 49 0e ee 82 2f cb 97 a6 a5 ee

>>   00000010: 5f

>

> Yikes, I never really got around to test MORUS and AEGIS on a BE

> machine...  My mistake, sorry :/

>


No worries - this is brand new code so this is not entirely unexpected.

>>

>> Fixes: 396be41f16fd ("crypto: morus - Add generic MORUS AEAD implementations")

>> Cc: <stable@vger.kernel.org> # v4.18+

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>

> Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>

>


Thanks!

>> ---

>>  crypto/morus1280.c |  7 ++-----

>>  crypto/morus640.c  | 16 ++++------------

>>  2 files changed, 6 insertions(+), 17 deletions(-)

>>

>> diff --git a/crypto/morus1280.c b/crypto/morus1280.c

>> index d057cf5ac4a8..3889c188f266 100644

>> --- a/crypto/morus1280.c

>> +++ b/crypto/morus1280.c

>> @@ -385,14 +385,11 @@ static void crypto_morus1280_final(struct morus1280_state *state,

>>                                    struct morus1280_block *tag_xor,

>>                                    u64 assoclen, u64 cryptlen)

>>  {

>> -       u64 assocbits = assoclen * 8;

>> -       u64 cryptbits = cryptlen * 8;

>> -

>>         struct morus1280_block tmp;

>>         unsigned int i;

>>

>> -       tmp.words[0] = cpu_to_le64(assocbits);

>> -       tmp.words[1] = cpu_to_le64(cryptbits);

>> +       tmp.words[0] = assoclen * 8;

>> +       tmp.words[1] = cryptlen * 8;

>>         tmp.words[2] = 0;

>>         tmp.words[3] = 0;

>>

>> diff --git a/crypto/morus640.c b/crypto/morus640.c

>> index 1ca76e54281b..da06ec2f6a80 100644

>> --- a/crypto/morus640.c

>> +++ b/crypto/morus640.c

>> @@ -384,21 +384,13 @@ static void crypto_morus640_final(struct morus640_state *state,

>>                                   struct morus640_block *tag_xor,

>>                                   u64 assoclen, u64 cryptlen)

>>  {

>> -       u64 assocbits = assoclen * 8;

>> -       u64 cryptbits = cryptlen * 8;

>> -

>> -       u32 assocbits_lo = (u32)assocbits;

>> -       u32 assocbits_hi = (u32)(assocbits >> 32);

>> -       u32 cryptbits_lo = (u32)cryptbits;

>> -       u32 cryptbits_hi = (u32)(cryptbits >> 32);

>> -

>>         struct morus640_block tmp;

>>         unsigned int i;

>>

>> -       tmp.words[0] = cpu_to_le32(assocbits_lo);

>> -       tmp.words[1] = cpu_to_le32(assocbits_hi);

>> -       tmp.words[2] = cpu_to_le32(cryptbits_lo);

>> -       tmp.words[3] = cpu_to_le32(cryptbits_hi);

>> +       tmp.words[0] = lower_32_bits(assoclen * 8);

>> +       tmp.words[1] = upper_32_bits(assoclen * 8);

>> +       tmp.words[2] = lower_32_bits(cryptlen * 8);

>> +       tmp.words[3] = upper_32_bits(cryptlen * 8);

>>

>>         for (i = 0; i < MORUS_BLOCK_WORDS; i++)

>>                 state->s[4].words[i] ^= state->s[0].words[i];

>> --

>> 2.19.0

>>

>

> Thanks,

>

> --

> Ondrej Mosnacek <omosnace at redhat dot com>

> Associate Software Engineer, Security Technologies

> Red Hat, Inc.
diff mbox series

Patch

diff --git a/crypto/morus1280.c b/crypto/morus1280.c
index d057cf5ac4a8..3889c188f266 100644
--- a/crypto/morus1280.c
+++ b/crypto/morus1280.c
@@ -385,14 +385,11 @@  static void crypto_morus1280_final(struct morus1280_state *state,
 				   struct morus1280_block *tag_xor,
 				   u64 assoclen, u64 cryptlen)
 {
-	u64 assocbits = assoclen * 8;
-	u64 cryptbits = cryptlen * 8;
-
 	struct morus1280_block tmp;
 	unsigned int i;
 
-	tmp.words[0] = cpu_to_le64(assocbits);
-	tmp.words[1] = cpu_to_le64(cryptbits);
+	tmp.words[0] = assoclen * 8;
+	tmp.words[1] = cryptlen * 8;
 	tmp.words[2] = 0;
 	tmp.words[3] = 0;
 
diff --git a/crypto/morus640.c b/crypto/morus640.c
index 1ca76e54281b..da06ec2f6a80 100644
--- a/crypto/morus640.c
+++ b/crypto/morus640.c
@@ -384,21 +384,13 @@  static void crypto_morus640_final(struct morus640_state *state,
 				  struct morus640_block *tag_xor,
 				  u64 assoclen, u64 cryptlen)
 {
-	u64 assocbits = assoclen * 8;
-	u64 cryptbits = cryptlen * 8;
-
-	u32 assocbits_lo = (u32)assocbits;
-	u32 assocbits_hi = (u32)(assocbits >> 32);
-	u32 cryptbits_lo = (u32)cryptbits;
-	u32 cryptbits_hi = (u32)(cryptbits >> 32);
-
 	struct morus640_block tmp;
 	unsigned int i;
 
-	tmp.words[0] = cpu_to_le32(assocbits_lo);
-	tmp.words[1] = cpu_to_le32(assocbits_hi);
-	tmp.words[2] = cpu_to_le32(cryptbits_lo);
-	tmp.words[3] = cpu_to_le32(cryptbits_hi);
+	tmp.words[0] = lower_32_bits(assoclen * 8);
+	tmp.words[1] = upper_32_bits(assoclen * 8);
+	tmp.words[2] = lower_32_bits(cryptlen * 8);
+	tmp.words[3] = upper_32_bits(cryptlen * 8);
 
 	for (i = 0; i < MORUS_BLOCK_WORDS; i++)
 		state->s[4].words[i] ^= state->s[0].words[i];