From 90b5011ba4b0ca681d1730f6146133be17553759 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Thu, 14 May 2026 17:41:12 -0500 Subject: [PATCH 1/2] wolfcrypt/src/wc_port.c and wolfssl/wolfcrypt/wc_port.h: add * wc_local_InitUp() * wc_local_InitUpDone() * wc_local_InitDown() * wc_local_InitDownDone() * wc_init_state_t * WC_DECLARE_INIT_STATE() * WC_INIT_STATE_* * union wc_init_state_bitfields * WC_INIT_STATE_RAISE_BAD_STATE(); fix&refactor thread safety mechanisms in wolfCrypt_Init() and wolfCrypt_Cleanup(), and fix a few preexisting error-handling flubs in wolfCrypt_Init(). --- .wolfssl_known_macro_extras | 3 +- wolfcrypt/src/wc_port.c | 335 ++++++++++++++++++++++++++++-------- wolfssl/wolfcrypt/wc_port.h | 40 +++++ 3 files changed, 308 insertions(+), 70 deletions(-) diff --git a/.wolfssl_known_macro_extras b/.wolfssl_known_macro_extras index 583422bea44..41296124a97 100644 --- a/.wolfssl_known_macro_extras +++ b/.wolfssl_known_macro_extras @@ -659,6 +659,7 @@ WC_FORCE_LINUXKM_FORTIFY_SOURCE WC_HASH_CUSTOM_MAX_BLOCK_SIZE WC_HASH_CUSTOM_MAX_DIGEST_SIZE WC_HASH_CUSTOM_MIN_DIGEST_SIZE +WC_INIT_BUSY_WHEN_CONTENDED WC_MLKEM_KERNEL_ASM WC_NO_ASYNC_SLEEP WC_NO_RNG_SIMPLE @@ -967,8 +968,6 @@ WOLFSSL_XIL_MSG_NO_SLEEP WOLFSSL_ZEPHYR WOLF_ALLOW_BUILTIN WOLF_CRYPTO_CB_CMD -WOLF_CRYPTO_CB_ONLY_ECC -WOLF_CRYPTO_CB_ONLY_RSA WOLF_CRYPTO_DEV WOLF_NO_TRAILING_ENUM_COMMAS WindowsCE diff --git a/wolfcrypt/src/wc_port.c b/wolfcrypt/src/wc_port.c index 04a0c54bf81..b3f10e62f85 100644 --- a/wolfcrypt/src/wc_port.c +++ b/wolfcrypt/src/wc_port.c @@ -241,12 +241,152 @@ Threading/Mutex options: #endif #endif -/* prevent multiple mutex initializations */ -#ifdef WOLFSSL_ATOMIC_OPS - wolfSSL_Atomic_Int initRefCount = WOLFSSL_ATOMIC_INITIALIZER(0); -#else - static int initRefCount = 0; -#endif + +/* Internal APIs for counting initialization depth, with initialization/cleanup + * races fully mitigated + */ +int wc_local_InitUp(wc_init_state_t *s, int doWait) +{ + union wc_init_state_bitfields exp_wc_init_state, new_wc_init_state; + exp_wc_init_state.u = WOLFSSL_ATOMIC_LOAD(*s); + + /* mitigate races on init/shutdown by looping. */ + for (;;) { + if (exp_wc_init_state.c.count == 0x1fffffff) + return SEQ_OVERFLOW_E; + new_wc_init_state = exp_wc_init_state; + if (exp_wc_init_state.c.state == WC_INIT_STATE_UNINITED) { + if (exp_wc_init_state.c.count != 0) + return BAD_STATE_E; + new_wc_init_state.c.state = WC_INIT_STATE_INITING; + } + else if (exp_wc_init_state.c.state == WC_INIT_STATE_BAD_STATE) { + return BAD_STATE_E; + } + else { + if (exp_wc_init_state.c.count == 0) + return BAD_STATE_E; + /* Force expected state to _INITED -- if actual value upon cmpxchg + * doesn't match (normally either _INITING or _CLEANING_UP), we'll + * spin until the transient state resolves to _INITED or _UNINITED + * (when the competing thread calls wc_local_InitUpDone() or + * wc_local_InitDownDone(), respectively). + */ + exp_wc_init_state.c.state = WC_INIT_STATE_INITED; + new_wc_init_state.c.state = WC_INIT_STATE_INITED; + } + ++new_wc_init_state.c.count; + /* if another thread entered _STATE_INITING or _CLEANING_UP, this will + * fail and spin. + */ + if (wolfSSL_Atomic_Uint_CompareExchange(s, + &exp_wc_init_state.u, + new_wc_init_state.u)) + break; + if (! doWait) + return BUSY_E; + WC_RELAX_LONG_LOOP(); /* not really long. */ + } + return new_wc_init_state.c.state; +} + +int wc_local_InitUpDone(wc_init_state_t *s) +{ + union wc_init_state_bitfields cur_wc_init_state; + cur_wc_init_state.u = WOLFSSL_ATOMIC_LOAD(*s); + if (cur_wc_init_state.c.state != WC_INIT_STATE_INITING) + return BAD_FUNC_ARG; + cur_wc_init_state.c.state = WC_INIT_STATE_INITED; + /* Note, because WC_INIT_STATE_INITING functions as a mutex on the module + * state, we can use a plain _STORE() to release the module into its _INITED + * state. + */ + WOLFSSL_ATOMIC_STORE(*s, cur_wc_init_state.u); + return 0; +} + +int wc_local_InitDown(wc_init_state_t *s, int doWait) +{ + union wc_init_state_bitfields exp_wc_init_state, new_wc_init_state; + + exp_wc_init_state.u = WOLFSSL_ATOMIC_LOAD(*s); + + /* mitigate races on init/shutdown by looping. */ + for (;;) { + if (exp_wc_init_state.c.state == WC_INIT_STATE_BAD_STATE) { + return BAD_STATE_E; + } + else if (exp_wc_init_state.c.state == WC_INIT_STATE_UNINITED) { + if (exp_wc_init_state.c.count == 0) { + /* thread attempted to wc_local_InitDown() without a matching + * previous wc_local_InitUp(). + */ + return ALREADY_E; /* backward compat */ + } + else { + /* nonzero .count with _STATE_UNINITED is impossible. */ + return BAD_STATE_E; + } + } + else if (exp_wc_init_state.c.state == WC_INIT_STATE_INITING) { + /* _INITING is impossible here unless a thread calls + * wc_local_InitDown() before (or without) successfully calling + * wc_local_InitUpDone(). + */ + return BAD_FUNC_ARG; + } + else if (exp_wc_init_state.c.state == WC_INIT_STATE_CLEANING_UP) { + if (exp_wc_init_state.c.count == 1) { + /* thread attempted to wc_local_InitDown() without a matching + * previous wc_local_InitUp(). + */ + return ALREADY_E; /* backward compat */ + } + else { + /* _CLEANING_UP is impossible with .count != 1. */ + return BAD_STATE_E; + } + } + else if (exp_wc_init_state.c.count == 0) { + /* zero count with state != _UNINITED is impossible. */ + return BAD_STATE_E; + } + new_wc_init_state = exp_wc_init_state; + if (exp_wc_init_state.c.count == 1) { + new_wc_init_state.c.state = WC_INIT_STATE_CLEANING_UP; + /* don't zero until end. */ + } + else + --new_wc_init_state.c.count; + if (wolfSSL_Atomic_Uint_CompareExchange(s, + &exp_wc_init_state.u, + new_wc_init_state.u)) + break; + if (! doWait) + return BUSY_E; + WC_RELAX_LONG_LOOP(); /* not really long. */ + } + + return new_wc_init_state.c.state; +} + +int wc_local_InitDownDone(wc_init_state_t *s) +{ + union wc_init_state_bitfields cur_wc_init_state; + cur_wc_init_state.u = WOLFSSL_ATOMIC_LOAD(*s); + if (cur_wc_init_state.c.state != WC_INIT_STATE_CLEANING_UP) + return BAD_FUNC_ARG; + cur_wc_init_state.c.state = WC_INIT_STATE_UNINITED; + cur_wc_init_state.c.count = 0; + /* Note, because WC_INIT_STATE_CLEANING_UP functions as a mutex on the + * module state, we can use a plain _STORE() to release the module into its + * _UNINITED state. + */ + WOLFSSL_ATOMIC_STORE(*s, cur_wc_init_state.u); + return 0; +} + +static WC_DECLARE_INIT_STATE(wolfcrypt_init_state); #if defined(__aarch64__) && defined(WOLFSSL_ARMASM_BARRIER_DETECT) int aarch64_use_sb = 0; @@ -258,11 +398,46 @@ int aarch64_use_sb = 0; WOLFSSL_ABI int wolfCrypt_Init(void) { - int ret = 0; - int my_initRefCount = wolfSSL_Atomic_Int_FetchAdd(&initRefCount, 1); - if (my_initRefCount == 0) { + int ret; +#if defined(HAVE_THREAD_LS) && !defined(NO_THREAD_LS) && defined(__GNUC__) + /* If thread-local storage is available, use it to prevent deadlock on + * recursion. We only do this when __GNUC__ -- this code is known to cause + * internal compiler faults on Watcom, and is probably problematic on other + * non-_GNUC__ targets besides. + */ + static THREAD_LS_T int in_init = 0; + if (in_init) + return DEADLOCK_AVERTED_E; + #define WOLFCRYPT_INIT_RAISE_BAD_STATE() do { \ + in_init = 0; \ + WC_INIT_STATE_RAISE_BAD_STATE(wolfcrypt_init_state); \ + return ret; \ + } while (0) +#else + #define WOLFCRYPT_INIT_RAISE_BAD_STATE() do { \ + WC_INIT_STATE_RAISE_BAD_STATE(wolfcrypt_init_state); \ + return ret; \ + } while (0) +#endif + + ret = wc_local_InitUp(&wolfcrypt_init_state, +#ifdef WC_INIT_BUSY_WHEN_CONTENDED + 0 +#else + 1 +#endif + ); + if (ret < 0) + return ret; + else if (ret == WC_INIT_STATE_INITED) + return 0; + else { WOLFSSL_ENTER("wolfCrypt_Init"); +#if defined(HAVE_THREAD_LS) && !defined(NO_THREAD_LS) && defined(__GNUC__) + in_init = 1; +#endif + #if defined(__aarch64__) && defined(WOLFSSL_ARMASM_BARRIER_DETECT) aarch64_use_sb = IS_AARCH64_SB(cpuid_get_flags()); #endif @@ -304,8 +479,8 @@ int wolfCrypt_Init(void) if( ret != TSIP_SUCCESS ) { WOLFSSL_MSG("RENESAS TSIP Open failed"); /* not return 1 since WOLFSSL_SUCCESS=1*/ - ret = -1;/* FATAL ERROR */ - return ret; + ret = WC_FAILURE; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -314,8 +489,8 @@ int wolfCrypt_Init(void) if( ret != 0 ) { WOLFSSL_MSG("Renesas RX64 HW Open failed"); /* not return 1 since WOLFSSL_SUCCESS=1*/ - ret = -1;/* FATAL ERROR */ - return ret; + ret = WC_FAILURE; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -324,8 +499,8 @@ int wolfCrypt_Init(void) if( ret != FSP_SUCCESS ) { WOLFSSL_MSG("RENESAS SCE Open failed"); /* not return 1 since WOLFSSL_SUCCESS=1*/ - ret = -1;/* FATAL ERROR */ - return ret; + ret = WC_FAILURE; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -333,7 +508,7 @@ int wolfCrypt_Init(void) ret = InitMemoryTracker(); if (ret != 0) { WOLFSSL_MSG("InitMemoryTracker failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -341,7 +516,7 @@ int wolfCrypt_Init(void) ret = allocate_wolfcrypt_linuxkm_fpu_states(); if (ret != 0) { WOLFSSL_MSG("allocate_wolfcrypt_linuxkm_fpu_states failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -350,7 +525,7 @@ int wolfCrypt_Init(void) ret = wolfSSL_CryptHwMutexInit(); if (ret != 0) { WOLFSSL_MSG("Hw crypt mutex init failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -360,7 +535,7 @@ int wolfCrypt_Init(void) ret = wc_DrbgState_MutexInit(); if (ret != 0) { WOLFSSL_MSG("DRBG state mutex init failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -368,7 +543,7 @@ int wolfCrypt_Init(void) ret = ksdk_port_init(); if (ret != 0) { WOLFSSL_MSG("KSDK port init failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -376,15 +551,16 @@ int wolfCrypt_Init(void) #if defined(MAX3266X_AES) && defined(WOLF_CRYPTO_CB) ret = wc_CryptoCb_RegisterDevice(WOLFSSL_MAX3266X_DEVID, wc_MxcCryptoCb, NULL); - if(ret != 0) { - return ret; + if (ret != 0) { + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #if defined(MAX3266X_RTC) ret = wc_MXC_RTC_Init(); if (ret != 0) { WOLFSSL_MSG("MXC RTC Init Failed"); - return WC_HW_E; + ret = WC_HW_E; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -393,7 +569,7 @@ int wolfCrypt_Init(void) ret = atmel_init(); if (ret != 0) { WOLFSSL_MSG("CryptoAuthLib init failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #if defined(WOLFSSL_CRYPTOCELL) @@ -401,28 +577,28 @@ int wolfCrypt_Init(void) ret = cc310_Init(); if (ret != 0) { WOLFSSL_MSG("CRYPTOCELL init failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #ifdef WOLFSSL_STSAFE ret = stsafe_interface_init(); if (ret != 0) { WOLFSSL_MSG("STSAFE init failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #if defined(WOLFSSL_TROPIC01) ret = Tropic01_Init(); if (ret != 0) { WOLFSSL_MSG("Tropic01 init failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #if defined(WOLFSSL_PSOC6_CRYPTO) ret = psoc6_crypto_port_init(); if (ret != 0) { WOLFSSL_MSG("PSoC6 crypto engine init failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -430,20 +606,24 @@ int wolfCrypt_Init(void) ret = maxq10xx_port_init(); if (ret != 0) { WOLFSSL_MSG("MAXQ10xx port init failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #ifdef WOLFSSL_SILABS_SE_ACCEL /* init handles if it is already initialized */ ret = sl_se_init(); + if (ret != 0) { + WOLFSSL_MSG("SILABS_SE_ACCEL init failed"); + WOLFCRYPT_INIT_RAISE_BAD_STATE(); + } #endif #if defined(WOLFSSL_SE050) && defined(WOLFSSL_SE050_INIT) ret = wc_se050_init(NULL); if (ret != 0) { WOLFSSL_MSG("SE050 init failed"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -462,13 +642,14 @@ int wolfCrypt_Init(void) #if defined(OPENSSL_EXTRA) || defined(DEBUG_WOLFSSL_VERBOSE) if ((ret = wc_LoggingInit()) != 0) { WOLFSSL_MSG("Error creating logging mutex"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #if defined(WOLFSSL_HAVE_PSA) - if ((ret = wc_psa_init()) != 0) - return ret; + if ((ret = wc_psa_init()) != 0) { + WOLFCRYPT_INIT_RAISE_BAD_STATE(); + } #endif #if defined(USE_WINDOWS_API) && defined(WIN_REUSE_CRYPT_HANDLE) @@ -482,7 +663,7 @@ int wolfCrypt_Init(void) ret = Entropy_Init(); if (ret != 0) { WOLFSSL_MSG("Error initializing entropy"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif @@ -493,14 +674,14 @@ int wolfCrypt_Init(void) #ifdef ECC_CACHE_CURVE if ((ret = wc_ecc_curve_cache_init()) != 0) { WOLFSSL_MSG("Error creating curve cache"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #if defined(HAVE_OID_ENCODING) && (!defined(HAVE_FIPS) || \ (defined(FIPS_VERSION_GE) && FIPS_VERSION_GE(6,0))) if ((ret = wc_ecc_oid_cache_init()) != 0) { WOLFSSL_MSG("Error creating ECC oid cache"); - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #endif @@ -514,69 +695,67 @@ int wolfCrypt_Init(void) } if (ret != SSP_SUCCESS) { WOLFSSL_MSG("Error opening SCE"); - return -1; /* FATAL_ERROR */ + ret = WC_FAILURE; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #if defined(WOLFSSL_DEVCRYPTO) if ((ret = wc_DevCryptoInit()) != 0) { - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #if defined(WOLFSSL_CAAM) if ((ret = wc_caamInit()) != 0) { - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #if defined(HAVE_ARIA) if ((ret = wc_AriaInit()) != 0) { - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #ifdef WOLFSSL_IMXRT_DCP if ((ret = wc_dcp_init()) != 0) { - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #ifdef WOLFSSL_NXP_CASPER if ((ret = wc_casper_init()) != 0) { - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #ifdef WOLFSSL_NXP_HASHCRYPT if ((ret = wc_hashcrypt_init()) != 0) { - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif #if defined(WOLFSSL_DSP) && !defined(WOLFSSL_DSP_BUILD) if ((ret = wolfSSL_InitHandle()) != 0) { - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } rpcmem_init(); #endif #if defined(HAVE_LIBOQS) if ((ret = wolfSSL_liboqsInit()) != 0) { - return ret; + WOLFCRYPT_INIT_RAISE_BAD_STATE(); } #endif - /* increment to 2, to signify successful initialization: */ - (void)wolfSSL_Atomic_Int_FetchAdd(&initRefCount, 1); - } - else { - if (my_initRefCount < 2) { - (void)wolfSSL_Atomic_Int_FetchSub(&initRefCount, 1); - ret = BUSY_E; - } - } +#undef WOLFCRYPT_INIT_RAISE_BAD_STATE - return ret; +#if defined(HAVE_THREAD_LS) && !defined(NO_THREAD_LS) && defined(__GNUC__) + in_init = 0; +#endif + return wc_local_InitUpDone(&wolfcrypt_init_state); + } + /* not reached */ } #if defined(WOLFSSL_TRACK_MEMORY_VERBOSE) && !defined(WOLFSSL_STATIC_MEMORY) @@ -597,10 +776,21 @@ long wolfCrypt_heap_peakBytes_checkpoint(void) { WOLFSSL_ABI int wolfCrypt_Cleanup(void) { - int ret = 0; - int my_initRefCount = wolfSSL_Atomic_Int_SubFetch(&initRefCount, 1); + int ret = wc_local_InitDown(&wolfcrypt_init_state, 1); + if (ret < 0) { + if (ret == WC_NO_ERR_TRACE(ALREADY_E)) + WOLFSSL_MSG("wolfCrypt_Cleanup() called with initRefCount <= 0."); + else if (ret == WC_NO_ERR_TRACE(BAD_STATE_E)) + WOLFSSL_MSG("wolfCrypt_Cleanup() failed: bad internal state."); + else + WOLFSSL_MSG("wolfCrypt_Cleanup() failed with unexpected error."); + return ret; + } + else if (ret == WC_INIT_STATE_INITED) + return 0; + else { + ret = 0; - if (my_initRefCount == 1) { WOLFSSL_ENTER("wolfCrypt_Cleanup"); #ifdef HAVE_ECC @@ -617,7 +807,11 @@ int wolfCrypt_Cleanup(void) #endif /* HAVE_ECC */ #if defined(OPENSSL_EXTRA) || defined(DEBUG_WOLFSSL_VERBOSE) - ret = wc_LoggingCleanup(); + { + int ret2 = wc_LoggingCleanup(); + if (ret == 0) + ret = ret2; + } #endif #if defined(WOLFSSL_TRACK_MEMORY) && !defined(WOLFSSL_STATIC_MEMORY) @@ -651,7 +845,11 @@ int wolfCrypt_Cleanup(void) cc310_Free(); #endif #ifdef WOLFSSL_SILABS_SE_ACCEL - ret = sl_se_deinit(); + { + int ret2 = sl_se_deinit(); + if (ret == 0) + ret = ret2; + } #endif #if defined(WOLFSSL_TROPIC01) Tropic01_Deinit(); @@ -697,19 +895,20 @@ int wolfCrypt_Cleanup(void) wc_MemZero_Free(); #endif - (void)wolfSSL_Atomic_Int_SubFetch(&initRefCount, 1); - #if defined(HAVE_LIBOQS) wolfSSL_liboqsClose(); #endif - } - else if (my_initRefCount < 0) { - (void)wolfSSL_Atomic_Int_AddFetch(&initRefCount, 1); - WOLFSSL_MSG("wolfCrypt_Cleanup() called with initRefCount <= 0."); - ret = ALREADY_E; + + { + int ret2 = wc_local_InitDownDone(&wolfcrypt_init_state); + if (ret == 0) + ret = ret2; + } + + return ret; } - return ret; + /* not reached */ } #ifndef NO_FILESYSTEM diff --git a/wolfssl/wolfcrypt/wc_port.h b/wolfssl/wolfcrypt/wc_port.h index d722ea8d104..ff3550cb3f4 100644 --- a/wolfssl/wolfcrypt/wc_port.h +++ b/wolfssl/wolfcrypt/wc_port.h @@ -941,6 +941,46 @@ WOLFSSL_API int wc_SetMutexCb(mutex_cb* cb); WOLFSSL_API mutex_cb* wc_GetMutexCb(void); #endif +/* Internal APIs for counting initialization depth, with initialization/cleanup + * races fully mitigated + */ +#ifdef WOLFSSL_ATOMIC_OPS + typedef wolfSSL_Atomic_Uint wc_init_state_t; + #define WC_INIT_STATE_INITIALIZER WOLFSSL_ATOMIC_INITIALIZER(0) +#else + typedef unsigned int wc_init_state_t; + #define WC_INIT_STATE_INITIALIZER 0 +#endif +#define WC_DECLARE_INIT_STATE(x) wc_init_state_t x = WC_INIT_STATE_INITIALIZER +#define WC_INIT_STATE_UNINITED 0U +#define WC_INIT_STATE_INITING 1U +#define WC_INIT_STATE_INITED 2U +#define WC_INIT_STATE_CLEANING_UP 3U +#define WC_INIT_STATE_BAD_STATE 4U +union wc_init_state_bitfields { + unsigned int u; + struct { + unsigned int state:3; + unsigned int count:29; + } c; +}; +/* Modules with no provisions for cleanup after a partially successful init need + * to enter a degraded state, returning BAD_STATE_E to the caller, signaling + * that restart is needed. This macro should only be called while + * _STATE_INITING (after wc_local_InitUp() returns _STATE_INITING and before + * wc_local_InitUpDone()), to assure the store is uncontended. + */ +#define WC_INIT_STATE_RAISE_BAD_STATE(x) do { \ + union wc_init_state_bitfields _x; \ + _x.u = WOLFSSL_ATOMIC_LOAD(x); \ + _x.c.state = WC_INIT_STATE_BAD_STATE; \ + WOLFSSL_ATOMIC_STORE(x, _x.u); \ + } while (0) +WOLFSSL_LOCAL int wc_local_InitUp(wc_init_state_t *s, int doWait); +WOLFSSL_LOCAL int wc_local_InitUpDone(wc_init_state_t *s); +WOLFSSL_LOCAL int wc_local_InitDown(wc_init_state_t *s, int doWait); +WOLFSSL_LOCAL int wc_local_InitDownDone(wc_init_state_t *s); + /* main crypto initialization function */ WOLFSSL_ABI WOLFSSL_API int wolfCrypt_Init(void); WOLFSSL_ABI WOLFSSL_API int wolfCrypt_Cleanup(void); From f1c0935b279ba1c332439195c1d3182104853e86 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Thu, 14 May 2026 22:16:45 -0500 Subject: [PATCH 2/2] wolfssl/wolfcrypt/wc_port.h: add WC_ATOMIC_INT_ARG and WC_ATOMIC_UINT_ARG macros, pivoting on WC_16BIT_CPU, and use them to assure operands to atomic operators are 32 bits, and wc_init_state_t is 32 bits, even on 16 bit targets like Arduino. --- wolfcrypt/src/wc_port.c | 7 +- wolfssl/wolfcrypt/wc_port.h | 200 +++++++++++++++++++++--------------- 2 files changed, 122 insertions(+), 85 deletions(-) diff --git a/wolfcrypt/src/wc_port.c b/wolfcrypt/src/wc_port.c index b3f10e62f85..9a2684f57bf 100644 --- a/wolfcrypt/src/wc_port.c +++ b/wolfcrypt/src/wc_port.c @@ -252,8 +252,13 @@ int wc_local_InitUp(wc_init_state_t *s, int doWait) /* mitigate races on init/shutdown by looping. */ for (;;) { - if (exp_wc_init_state.c.count == 0x1fffffff) + wc_static_assert(WC_INIT_STATE_STATE_BITS + WC_INIT_STATE_COUNT_BITS == + sizeof(WC_ATOMIC_UINT_ARG) * 8); + if (exp_wc_init_state.c.count == + ((1U << WC_INIT_STATE_COUNT_BITS) - 1U)) + { return SEQ_OVERFLOW_E; + } new_wc_init_state = exp_wc_init_state; if (exp_wc_init_state.c.state == WC_INIT_STATE_UNINITED) { if (exp_wc_init_state.c.count != 0) diff --git a/wolfssl/wolfcrypt/wc_port.h b/wolfssl/wolfcrypt/wc_port.h index ff3550cb3f4..025bae053fb 100644 --- a/wolfssl/wolfcrypt/wc_port.h +++ b/wolfssl/wolfcrypt/wc_port.h @@ -516,6 +516,14 @@ typedef wolfSSL_Mutex wolfSSL_RwLock; #endif +#ifdef WC_16BIT_CPU + #define WC_ATOMIC_INT_ARG long int + #define WC_ATOMIC_UINT_ARG long unsigned int +#else + #define WC_ATOMIC_INT_ARG int + #define WC_ATOMIC_UINT_ARG unsigned int +#endif + #ifndef WOLFSSL_NO_ATOMICS #if defined(WOLFSSL_USER_DEFINED_ATOMICS) /* user-supplied bindings for wolfSSL_Atomic_Int etc. */ @@ -527,8 +535,8 @@ #define WOLFSSL_ATOMIC_OPS #endif #elif defined(SINGLE_THREADED) - typedef int wolfSSL_Atomic_Int; - typedef unsigned int wolfSSL_Atomic_Uint; + typedef WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int; + typedef WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint; #define WOLFSSL_ATOMIC_INITIALIZER(x) (x) #define WOLFSSL_ATOMIC_LOAD(x) (x) #define WOLFSSL_ATOMIC_STORE(x, val) (x) = (val) @@ -591,10 +599,10 @@ * Allows a user-supplied override definition with type introspection. */ #ifndef WOLFSSL_ATOMIC_COERCE_INT - #define WOLFSSL_ATOMIC_COERCE_INT(x) ((int)(x)) + #define WOLFSSL_ATOMIC_COERCE_INT(x) ((WC_ATOMIC_INT_ARG)(x)) #endif #ifndef WOLFSSL_ATOMIC_COERCE_UINT - #define WOLFSSL_ATOMIC_COERCE_UINT(x) ((unsigned int)(x)) + #define WOLFSSL_ATOMIC_COERCE_UINT(x) ((WC_ATOMIC_UINT_ARG)(x)) #endif #ifdef WOLFSSL_USER_DEFINED_ATOMICS @@ -602,32 +610,39 @@ * wolfSSL_Atomic_Int_FetchAdd(), etc. */ #elif defined(WOLFSSL_ATOMIC_OPS) && !defined(SINGLE_THREADED) - WOLFSSL_API void wolfSSL_Atomic_Int_Init(wolfSSL_Atomic_Int* c, int i); + WOLFSSL_API void wolfSSL_Atomic_Int_Init(wolfSSL_Atomic_Int* c, + WC_ATOMIC_INT_ARG i); WOLFSSL_API void wolfSSL_Atomic_Uint_Init( - wolfSSL_Atomic_Uint* c, unsigned int i); + wolfSSL_Atomic_Uint* c, WC_ATOMIC_UINT_ARG i); /* FetchOp functions return the value of the counter immediately preceding * the effects of the operation. * OpFetch functions return the value of the counter immediately after * the effects of the operation. */ - WOLFSSL_API int wolfSSL_Atomic_Int_FetchAdd(wolfSSL_Atomic_Int* c, int i); - WOLFSSL_API int wolfSSL_Atomic_Int_FetchSub(wolfSSL_Atomic_Int* c, int i); - WOLFSSL_API int wolfSSL_Atomic_Int_AddFetch(wolfSSL_Atomic_Int* c, int i); - WOLFSSL_API int wolfSSL_Atomic_Int_SubFetch(wolfSSL_Atomic_Int* c, int i); - WOLFSSL_API int wolfSSL_Atomic_Int_Exchange( - wolfSSL_Atomic_Int* c, int new_i); + WOLFSSL_API WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_FetchAdd( + wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG i); + WOLFSSL_API WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_FetchSub( + wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG i); + WOLFSSL_API WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_AddFetch( + wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG i); + WOLFSSL_API WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_SubFetch( + wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG i); + WOLFSSL_API WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_Exchange( + wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG new_i); WOLFSSL_API int wolfSSL_Atomic_Int_CompareExchange( - wolfSSL_Atomic_Int* c, int *expected_i, int new_i); - WOLFSSL_API unsigned int wolfSSL_Atomic_Uint_FetchAdd( - wolfSSL_Atomic_Uint* c, unsigned int i); - WOLFSSL_API unsigned int wolfSSL_Atomic_Uint_FetchSub( - wolfSSL_Atomic_Uint* c, unsigned int i); - WOLFSSL_API unsigned int wolfSSL_Atomic_Uint_AddFetch( - wolfSSL_Atomic_Uint* c, unsigned int i); - WOLFSSL_API unsigned int wolfSSL_Atomic_Uint_SubFetch( - wolfSSL_Atomic_Uint* c, unsigned int i); + wolfSSL_Atomic_Int* c, WC_ATOMIC_INT_ARG *expected_i, + WC_ATOMIC_INT_ARG new_i); + WOLFSSL_API WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_FetchAdd( + wolfSSL_Atomic_Uint* c, WC_ATOMIC_UINT_ARG i); + WOLFSSL_API WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_FetchSub( + wolfSSL_Atomic_Uint* c, WC_ATOMIC_UINT_ARG i); + WOLFSSL_API WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_AddFetch( + wolfSSL_Atomic_Uint* c, WC_ATOMIC_UINT_ARG i); + WOLFSSL_API WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_SubFetch( + wolfSSL_Atomic_Uint* c, WC_ATOMIC_UINT_ARG i); WOLFSSL_API int wolfSSL_Atomic_Uint_CompareExchange( - wolfSSL_Atomic_Uint* c, unsigned int *expected_i, unsigned int new_i); + wolfSSL_Atomic_Uint* c, WC_ATOMIC_UINT_ARG *expected_i, + WC_ATOMIC_UINT_ARG new_i); WOLFSSL_API int wolfSSL_Atomic_Ptr_CompareExchange( void* volatile * c, void **expected_ptr, void *new_ptr); #else @@ -637,33 +652,47 @@ * !defined(WOLFSSL_ATOMIC_OPS) && !defined(SINGLE_THREADED). This forces * local awareness of thread-unsafe semantics. */ + #define wolfSSL_Atomic_Int_Init(c, i) (*(c) = (i)) #define wolfSSL_Atomic_Uint_Init(c, i) (*(c) = (i)) - static WC_INLINE int wolfSSL_Atomic_Int_FetchAdd(int *c, int i) { - int ret = *c; + static WC_INLINE WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_FetchAdd( + WC_ATOMIC_INT_ARG *c, + WC_ATOMIC_INT_ARG i) + { + WC_ATOMIC_INT_ARG ret = *c; *c += i; return ret; } - static WC_INLINE int wolfSSL_Atomic_Int_FetchSub(int *c, int i) { - int ret = *c; + static WC_INLINE WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_FetchSub( + WC_ATOMIC_INT_ARG *c, + WC_ATOMIC_INT_ARG i) + { + WC_ATOMIC_INT_ARG ret = *c; *c -= i; return ret; } - static WC_INLINE int wolfSSL_Atomic_Int_AddFetch(int *c, int i) { + static WC_INLINE WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_AddFetch( + WC_ATOMIC_INT_ARG *c, + WC_ATOMIC_INT_ARG i) + { return (*c += i); } - static WC_INLINE int wolfSSL_Atomic_Int_SubFetch(int *c, int i) { + static WC_INLINE WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_SubFetch( + WC_ATOMIC_INT_ARG *c, + WC_ATOMIC_INT_ARG i) + { return (*c -= i); } - static WC_INLINE int wolfSSL_Atomic_Int_Exchange( - int *c, int new_i) + static WC_INLINE WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_Exchange( + WC_ATOMIC_INT_ARG *c, WC_ATOMIC_INT_ARG new_i) { - int ret = *c; + WC_ATOMIC_INT_ARG ret = *c; *c = new_i; return ret; } - static WC_INLINE int wolfSSL_Atomic_Int_CompareExchange( - int *c, int *expected_i, int new_i) + static WC_INLINE WC_ATOMIC_INT_ARG wolfSSL_Atomic_Int_CompareExchange( + WC_ATOMIC_INT_ARG *c, WC_ATOMIC_INT_ARG *expected_i, + WC_ATOMIC_INT_ARG new_i) { if (*c == *expected_i) { *c = new_i; @@ -686,32 +715,33 @@ return 0; } } - static WC_INLINE unsigned int wolfSSL_Atomic_Uint_FetchAdd( - unsigned int *c, unsigned int i) + static WC_INLINE WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_FetchAdd( + WC_ATOMIC_UINT_ARG *c, WC_ATOMIC_UINT_ARG i) { - unsigned int ret = *c; + WC_ATOMIC_UINT_ARG ret = *c; *c += i; return ret; } - static WC_INLINE unsigned int wolfSSL_Atomic_Uint_FetchSub( - unsigned int *c, unsigned int i) + static WC_INLINE WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_FetchSub( + WC_ATOMIC_UINT_ARG *c, WC_ATOMIC_UINT_ARG i) { - unsigned int ret = *c; + WC_ATOMIC_UINT_ARG ret = *c; *c -= i; return ret; } - static WC_INLINE unsigned int wolfSSL_Atomic_Uint_AddFetch( - unsigned int *c, unsigned int i) + static WC_INLINE WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_AddFetch( + WC_ATOMIC_UINT_ARG *c, WC_ATOMIC_UINT_ARG i) { return (*c += i); } - static WC_INLINE unsigned int wolfSSL_Atomic_Uint_SubFetch( - unsigned int *c, unsigned int i) + static WC_INLINE WC_ATOMIC_UINT_ARG wolfSSL_Atomic_Uint_SubFetch( + WC_ATOMIC_UINT_ARG *c, WC_ATOMIC_UINT_ARG i) { return (*c -= i); } static WC_INLINE int wolfSSL_Atomic_Uint_CompareExchange( - unsigned int *c, unsigned int *expected_i, unsigned int new_i) + WC_ATOMIC_UINT_ARG *c, WC_ATOMIC_UINT_ARG *expected_i, + WC_ATOMIC_UINT_ARG new_i) { if (*c == *expected_i) { *c = new_i; @@ -724,6 +754,48 @@ } #endif +/* Internal APIs for counting initialization depth, with initialization/cleanup + * races fully mitigated + */ +#ifdef WOLFSSL_ATOMIC_OPS + typedef wolfSSL_Atomic_Uint wc_init_state_t; + #define WC_INIT_STATE_INITIALIZER WOLFSSL_ATOMIC_INITIALIZER(0) +#else + typedef WC_ATOMIC_UINT_ARG wc_init_state_t; + #define WC_INIT_STATE_INITIALIZER 0 +#endif +#define WC_DECLARE_INIT_STATE(x) wc_init_state_t x = WC_INIT_STATE_INITIALIZER +#define WC_INIT_STATE_UNINITED 0U +#define WC_INIT_STATE_INITING 1U +#define WC_INIT_STATE_INITED 2U +#define WC_INIT_STATE_CLEANING_UP 3U +#define WC_INIT_STATE_BAD_STATE 4U +#define WC_INIT_STATE_STATE_BITS 3 +#define WC_INIT_STATE_COUNT_BITS 29 +union wc_init_state_bitfields { + WC_ATOMIC_UINT_ARG u; + struct { + WC_ATOMIC_UINT_ARG state:WC_INIT_STATE_STATE_BITS; + WC_ATOMIC_UINT_ARG count:WC_INIT_STATE_COUNT_BITS; + } c; +}; +/* Modules with no provisions for cleanup after a partially successful init need + * to enter a degraded state, returning BAD_STATE_E to the caller, signaling + * that restart is needed. This macro should only be called while + * _STATE_INITING (after wc_local_InitUp() returns _STATE_INITING and before + * wc_local_InitUpDone()), to assure the store is uncontended. + */ +#define WC_INIT_STATE_RAISE_BAD_STATE(x) do { \ + union wc_init_state_bitfields _x; \ + _x.u = WOLFSSL_ATOMIC_LOAD(x); \ + _x.c.state = WC_INIT_STATE_BAD_STATE; \ + WOLFSSL_ATOMIC_STORE(x, _x.u); \ + } while (0) +WOLFSSL_LOCAL int wc_local_InitUp(wc_init_state_t *s, int doWait); +WOLFSSL_LOCAL int wc_local_InitUpDone(wc_init_state_t *s); +WOLFSSL_LOCAL int wc_local_InitDown(wc_init_state_t *s, int doWait); +WOLFSSL_LOCAL int wc_local_InitDownDone(wc_init_state_t *s); + /* Reference counting. */ typedef struct wolfSSL_RefWithMutex { #if !defined(SINGLE_THREADED) @@ -941,46 +1013,6 @@ WOLFSSL_API int wc_SetMutexCb(mutex_cb* cb); WOLFSSL_API mutex_cb* wc_GetMutexCb(void); #endif -/* Internal APIs for counting initialization depth, with initialization/cleanup - * races fully mitigated - */ -#ifdef WOLFSSL_ATOMIC_OPS - typedef wolfSSL_Atomic_Uint wc_init_state_t; - #define WC_INIT_STATE_INITIALIZER WOLFSSL_ATOMIC_INITIALIZER(0) -#else - typedef unsigned int wc_init_state_t; - #define WC_INIT_STATE_INITIALIZER 0 -#endif -#define WC_DECLARE_INIT_STATE(x) wc_init_state_t x = WC_INIT_STATE_INITIALIZER -#define WC_INIT_STATE_UNINITED 0U -#define WC_INIT_STATE_INITING 1U -#define WC_INIT_STATE_INITED 2U -#define WC_INIT_STATE_CLEANING_UP 3U -#define WC_INIT_STATE_BAD_STATE 4U -union wc_init_state_bitfields { - unsigned int u; - struct { - unsigned int state:3; - unsigned int count:29; - } c; -}; -/* Modules with no provisions for cleanup after a partially successful init need - * to enter a degraded state, returning BAD_STATE_E to the caller, signaling - * that restart is needed. This macro should only be called while - * _STATE_INITING (after wc_local_InitUp() returns _STATE_INITING and before - * wc_local_InitUpDone()), to assure the store is uncontended. - */ -#define WC_INIT_STATE_RAISE_BAD_STATE(x) do { \ - union wc_init_state_bitfields _x; \ - _x.u = WOLFSSL_ATOMIC_LOAD(x); \ - _x.c.state = WC_INIT_STATE_BAD_STATE; \ - WOLFSSL_ATOMIC_STORE(x, _x.u); \ - } while (0) -WOLFSSL_LOCAL int wc_local_InitUp(wc_init_state_t *s, int doWait); -WOLFSSL_LOCAL int wc_local_InitUpDone(wc_init_state_t *s); -WOLFSSL_LOCAL int wc_local_InitDown(wc_init_state_t *s, int doWait); -WOLFSSL_LOCAL int wc_local_InitDownDone(wc_init_state_t *s); - /* main crypto initialization function */ WOLFSSL_ABI WOLFSSL_API int wolfCrypt_Init(void); WOLFSSL_ABI WOLFSSL_API int wolfCrypt_Cleanup(void);