RE: [PATCH v3 2/4] crypto: aria: do not use magic number offsets of aria_ctx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Taehee Yoo <ap420073@xxxxxxxxx>
> Sent: Sunday, November 6, 2022 8:36 AM
> To: linux-crypto@xxxxxxxxxxxxxxx; herbert@xxxxxxxxxxxxxxxxxxx;
> davem@xxxxxxxxxxxxx; tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; bp@xxxxxxxxx;
> dave.hansen@xxxxxxxxxxxxxxx; hpa@xxxxxxxxx;
> kirill.shutemov@xxxxxxxxxxxxxxx; richard@xxxxxx; viro@xxxxxxxxxxxxxxxxxx;
> sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx; jpoimboe@xxxxxxxxxx; Elliott,
> Robert (Servers) <elliott@xxxxxxx>; x86@xxxxxxxxxx; jussi.kivilinna@xxxxxx
> Cc: ap420073@xxxxxxxxx
> Subject: [PATCH v3 2/4] crypto: aria: do not use magic number offsets of
> aria_ctx
> 
> aria-avx assembly code accesses members of aria_ctx with magic number
> offset. If the shape of struct aria_ctx is changed carelessly,
> aria-avx will not work.
> So, we need to ensure accessing members of aria_ctx with correct
> offset values, not with magic numbers.
> 
> It adds ARIA_CTX_enc_key, ARIA_CTX_dec_key, and ARIA_CTX_rounds in the
> asm-offsets.c So, correct offset definitions will be generated.
> aria-avx assembly code can access members of aria_ctx safely with
> these definitions.
> 
> diff --git a/arch/x86/crypto/aria-aesni-avx-asm_64.S
...
> 
>  #include <linux/linkage.h>
>  #include <asm/frame.h>
> -
> -/* struct aria_ctx: */
> -#define enc_key 0
> -#define dec_key 272
> -#define rounds 544

That structure also has a key_length field after the rounds field.
aria_set_key() sets it, but no function ever seems to use it.
Perhaps that field should be removed?

> +#include <asm/asm-offsets.h>

That makes the offsets flexible, which is good.

The assembly code also implicitly assumes the size of each of those fields
(e.g., enc_key is 272 bytes, dec_key is 272 bytes, and rounds is 4 bytes).
A BUILD_BUG_ON confirming those assumptions might be worthwhile.

> diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> index cb50589a7102..32192a91c65b 100644
> --- a/arch/x86/kernel/asm-offsets.c
> +++ b/arch/x86/kernel/asm-offsets.c
> @@ -7,6 +7,7 @@
>  #define COMPILE_OFFSETS
> 
>  #include <linux/crypto.h>
> +#include <crypto/aria.h>

Is it safe to include .h files for a large number of crypto modules
in one C file? It seems like they could easily include naming conflicts. 

However, this set does seem to compile cleanly:

+// no .h for aegis, but ctx is simple
+#include <crypto/aes.h>
+#include <crypto/aria.h>
+#include <crypto/blake2s.h>
+#include <crypto/blowfish.h>
+// no .h for camellia in crypto; it is in arch/x86/crypto
+#include <crypto/cast5.h>
+#include <crypto/cast6.h>
+#include <crypto/chacha.h>
+// no .h for crc32c, crc32, crct10dif, but no ctx structure to worry about
+#include <crypto/curve25519.h>
+#include <crypto/des.h>
+#include <crypto/ghash.h>
+#include <crypto/nhpoly1305.h>
+#include <crypto/poly1305.h>
+#include <crypto/polyval.h>
+#include <crypto/serpent.h>
+#include <crypto/sha1.h>
+#include <crypto/sha2.h>
+#include <crypto/sm3.h>
+#include <crypto/twofish.h>







[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux