From 13c73a9691418613465f92f65ca195ba256fc2d7 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Wed, 2 Apr 2025 17:30:19 -0500 Subject: [PATCH] linuxkm/lkcapi_glue.c: add LINUXKM_LKCAPI_NEED_AES_COMMON_FUNCS and LINUXKM_LKCAPI_NEED_AES_SKCIPHER_COMMON_FUNCS helper macros (peer review suggestion). wolfcrypt/src/aes.c: add lengthy comment in software wc_AesSetKeyLocal() explaining the dynamics of aes->use_aesni (peer review suggestion), and in the !haveAESNI && WC_C_DYNAMIC_FALLBACK case, return with immediate success rather than following through to the redundant AesSetKey_C(). --- linuxkm/lkcapi_glue.c | 51 ++++++++++++++++++++++------------------- wolfcrypt/src/aes.c | 36 +++++++++++++++++++++++++++++ wolfssl/wolfcrypt/aes.h | 3 ++- 3 files changed, 65 insertions(+), 25 deletions(-) diff --git a/linuxkm/lkcapi_glue.c b/linuxkm/lkcapi_glue.c index c7ae88124..9bd437d76 100644 --- a/linuxkm/lkcapi_glue.c +++ b/linuxkm/lkcapi_glue.c @@ -210,6 +210,29 @@ static int disable_setkey_warnings = 0; static int linuxkm_test_aesecb(void); #endif +#if defined(LINUXKM_LKCAPI_REGISTER_AESCBC) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESCFB) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESCTR) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESOFB) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESECB) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESGCM) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESGCM_RFC4106) + #define LINUXKM_LKCAPI_NEED_AES_COMMON_FUNCS +#endif + +#if defined(LINUXKM_LKCAPI_REGISTER_AESCBC) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESCFB) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESCTR) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESOFB) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESECB) + #define LINUXKM_LKCAPI_NEED_AES_SKCIPHER_COMMON_FUNCS +#endif + +#if defined(LINUXKM_LKCAPI_REGISTER_AESGCM) || \ + defined(LINUXKM_LKCAPI_REGISTER_AESGCM_RFC4106) + #define LINUXKM_LKCAPI_REGISTER_AEADS +#endif + /* km_AesX(): wrappers to wolfcrypt wc_AesX functions and * structures. */ @@ -312,13 +335,7 @@ struct km_AesCtx { #endif }; -#if defined(LINUXKM_LKCAPI_REGISTER_AESCBC) || \ - defined(LINUXKM_LKCAPI_REGISTER_AESCFB) || \ - defined(LINUXKM_LKCAPI_REGISTER_AESGCM) || \ - defined(LINUXKM_LKCAPI_REGISTER_AESGCM_RFC4106) || \ - defined(LINUXKM_LKCAPI_REGISTER_AESCTR) || \ - defined(LINUXKM_LKCAPI_REGISTER_AESOFB) || \ - defined(LINUXKM_LKCAPI_REGISTER_AESECB) +#ifdef LINUXKM_LKCAPI_NEED_AES_COMMON_FUNCS static void km_AesExitCommon(struct km_AesCtx * ctx); @@ -518,11 +535,7 @@ static void km_AesExitCommon(struct km_AesCtx * ctx) #endif } -#if defined(LINUXKM_LKCAPI_REGISTER_AESCBC) || \ - defined(LINUXKM_LKCAPI_REGISTER_AESCFB) || \ - defined(LINUXKM_LKCAPI_REGISTER_AESCTR) || \ - defined(LINUXKM_LKCAPI_REGISTER_AESOFB) || \ - defined(LINUXKM_LKCAPI_REGISTER_AESECB) +#ifdef LINUXKM_LKCAPI_NEED_AES_SKCIPHER_COMMON_FUNCS static int km_AesSetKeyCommon(struct km_AesCtx * ctx, const u8 *in_key, unsigned int key_len, const char * name) @@ -595,19 +608,9 @@ static void km_AesExit(struct crypto_skcipher *tfm) km_AesExitCommon(ctx); } -#endif /* LINUXKM_LKCAPI_REGISTER_AESCBC || - * LINUXKM_LKCAPI_REGISTER_AESCFB || - * LINUXKM_LKCAPI_REGISTER_AESCTR || - * LINUXKM_LKCAPI_REGISTER_AESOFB || - * LINUXKM_LKCAPI_REGISTER_AESECB - */ +#endif /* LINUXKM_LKCAPI_NEED_AES_SKCIPHER_COMMON_FUNCS */ -#endif /* LINUXKM_LKCAPI_REGISTER_AESCBC || - * LINUXKM_LKCAPI_REGISTER_AESCFB || LINUXKM_LKCAPI_REGISTER_AESGCM || - * LINUXKM_LKCAPI_REGISTER_AESGCM_RFC4106 || - * LINUXKM_LKCAPI_REGISTER_AESCTR || LINUXKM_LKCAPI_REGISTER_AESOFB || - * LINUXKM_LKCAPI_REGISTER_AESECB - */ +#endif /* LINUXKM_LKCAPI_NEED_AES_COMMON_FUNCS */ #ifdef LINUXKM_LKCAPI_REGISTER_AESCBC diff --git a/wolfcrypt/src/aes.c b/wolfcrypt/src/aes.c index c0e3c69b2..bb2c4160c 100644 --- a/wolfcrypt/src/aes.c +++ b/wolfcrypt/src/aes.c @@ -4575,6 +4575,36 @@ static void AesSetKey_C(Aes* aes, const byte* key, word32 keySz, int dir) #endif /* WC_C_DYNAMIC_FALLBACK */ #ifdef WOLFSSL_AESNI + + /* The dynamics for determining whether AES-NI will be used are tricky. + * + * First, we check for CPU support and cache the result -- if AES-NI is + * missing, we always shortcut to the AesSetKey_C() path. + * + * Second, if the CPU supports AES-NI, we confirm on a per-call basis + * that it's safe to use in the caller context, using + * SAVE_VECTOR_REGISTERS2(). This is an always-true no-op in user-space + * builds, but has substantive logic behind it in kernel module builds. + * + * The outcome when SAVE_VECTOR_REGISTERS2() fails depends on + * WC_C_DYNAMIC_FALLBACK -- if that's defined, we return immediately with + * success but with AES-NI disabled (the earlier AesSetKey_C() allows + * future encrypt/decrypt calls to succeed), otherwise we fail. + * + * Upon successful return, aes->use_aesni will have a zero value if + * AES-NI is disabled, and a nonzero value if it's enabled. + * + * An additional, optional semantic is available via + * WC_FLAG_DONT_USE_AESNI, and is used in some kernel module builds to + * let the caller inhibit AES-NI. When this macro is defined, + * wc_AesInit() before wc_AesSetKey() is imperative, to avoid a read of + * uninitialized data in aes->use_aesni. That's why support for + * WC_FLAG_DONT_USE_AESNI must remain optional -- wc_AesInit() was only + * added in release 3.11.0, so legacy applications inevitably call + * wc_AesSetKey() on uninitialized Aes contexts. This must continue to + * function correctly with default build settings. + */ + if (checkedAESNI == 0) { haveAESNI = Check_CPU_support_AES(); checkedAESNI = 1; @@ -4627,6 +4657,12 @@ static void AesSetKey_C(Aes* aes, const byte* key, word32 keySz, int dir) } else { aes->use_aesni = 0; +#ifdef WC_C_DYNAMIC_FALLBACK + /* If WC_C_DYNAMIC_FALLBACK, we already called AesSetKey_C() + * above. + */ + return 0; +#endif } #endif /* WOLFSSL_AESNI */ diff --git a/wolfssl/wolfcrypt/aes.h b/wolfssl/wolfcrypt/aes.h index 5fd0e63aa..0ce8acc02 100644 --- a/wolfssl/wolfcrypt/aes.h +++ b/wolfssl/wolfcrypt/aes.h @@ -306,7 +306,8 @@ struct Aes { #if defined(WOLFSSL_LINUXKM) || defined(WC_WANT_FLAG_DONT_USE_AESNI) /* Note, we can't support WC_FLAG_DONT_USE_AESNI by default because we * need to support legacy applications that call wc_AesSetKey() on - * uninited struct Aes. + * uninited struct Aes. For details see the software implementation of + * wc_AesSetKeyLocal() (aes.c). */ #define WC_FLAG_DONT_USE_AESNI 2 #endif