> -----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>