From 1715608f6863287e1d4c50e69efcd1e4408e65ac Mon Sep 17 00:00:00 2001 From: Emma Stensland Date: Wed, 9 Sep 2026 13:21:19 -0600 Subject: [PATCH 1/3] F-7091, F-7419, F-8169: track entropy_mutex ownership before stopping the counter thread or unlocking --- tests/unit-mcdc/test_wolfentropy_whitebox.c | 28 +++++------ wolfcrypt/src/wolfentropy.c | 52 ++++++++++++++------- 2 files changed, 50 insertions(+), 30 deletions(-) diff --git a/tests/unit-mcdc/test_wolfentropy_whitebox.c b/tests/unit-mcdc/test_wolfentropy_whitebox.c index 4cb9eb36e6f..947818d628b 100644 --- a/tests/unit-mcdc/test_wolfentropy_whitebox.c +++ b/tests/unit-mcdc/test_wolfentropy_whitebox.c @@ -81,7 +81,7 @@ * those DO come from sha3.o in the archive. Refusing one of them is therefore * the reachable equivalent: Entropy_MemUse() -> Entropy_GetNoise() -> * Entropy_HealthTest_Startup() propagates it into `ret` before the sample - * loop at :781 is ever evaluated. mcdc_fault_hash.h does not carry SHA-3 (no + * loop is ever evaluated. mcdc_fault_hash.h does not carry SHA-3 (no * white-box has needed it before), so the wrapper is local to this TU. * * The wrapper is defined BEFORE the macro exists, so it still reaches the @@ -329,12 +329,12 @@ static void wb_get_loop_early_exit(void) * "for (i = 0; (ret == 0) && (i < ENTROPY_INITIAL_COUNT); i++)" * On a healthy host the loop always runs to completion, so idx0 ("ret == 0") * only ever shows TRUE and the loop is only ever left through idx1. `ret` is - * assignable inside the loop (the two health tests at :782-:786), but real + * assignable inside the loop (the two health tests), but real * MemUse jitter never trips REP_CUTOFF/PROP_CUTOFF, and the noise buffer is * filled by the file-static Entropy_GetNoise() which cannot be rigged * directly. The reachable lever is the one thing Entropy_GetNoise() depends on * from outside this file: refuse the conditioner's SHA3-256 update, so - * Entropy_MemUse() fails, Entropy_GetNoise() returns that error at :779, and + * Entropy_MemUse() fails, Entropy_GetNoise() returns that error, and * the loop condition is evaluated once with ret != 0 -- idx0 FALSE, the * missing half. The healthy call immediately before it (same binary) supplies * (T,T) and, at i == ENTROPY_INITIAL_COUNT, (T,F). */ @@ -392,24 +392,24 @@ static void wb_startup_noise_fail(void) /* ---- wc_Entropy_Get() mutex-failure vectors ------------------------------ * * - * 881: if ((ret == 0) && (wc_LockMutex(&entropy_mutex) != 0)) - * 893: if ((ret == 0) && ((prop_total == 0) || (!rep_have_prev))) + * LOCK: if (wc_LockMutex(&entropy_mutex) != 0) (inside "if (ret == 0)") + * PROP: if ((ret == 0) && ((prop_total == 0) || (!rep_have_prev))) * - * A live, correctly initialised mutex always locks, so 881 only ever shows - * (T,F) and 893 only ever shows its idx0 TRUE half. mcdc_fault_mutex.h + * A live, correctly initialised mutex always locks, so LOCK only ever shows + * FALSE and PROP only ever shows its idx0 TRUE half. mcdc_fault_mutex.h * redirects this TU's wc_LockMutex() through a hook; mcdc_fm_lock_once makes * the NEXT lock -- and only that one -- refuse: * - * armed -> 881 (T,T) -> ret = BAD_MUTEX_E, which then makes 893's idx0 + * armed -> LOCK TRUE -> ret = BAD_MUTEX_E, which then makes PROP's idx0 * FALSE at the very next decision (same call, same binary) - * unarmed -> 881 (T,F) and 893 idx0 TRUE + * unarmed -> LOCK FALSE and PROP idx0 TRUE * * A refused lock is the one path wc_Entropy_Get() must NOT unlock on, and it - * doesn't: the tail is guarded on the mutex error code, so no unlock of an - * unheld mutex happens and no global state is touched. The idx0 operand of - * 881 itself stays a justified residual: outside HAVE_FIPS builds nothing - * runs between "ret = 0" and this test, so ret is 0 by construction and the - * operand has no independence pair here. + * doesn't: the tail is guarded on `locked`, so no unlock of an + * unheld mutex happens and no global state is touched. The enclosing + * "ret == 0" guard's FALSE half stays a justified residual: outside HAVE_FIPS + * builds nothing runs between "ret = 0" and this test, so ret is 0 by + * construction. * ------------------------------------------------------------------------ */ #if defined(HAVE_ENTROPY_MEMUSE) && !defined(MCDC_FM_UNAVAILABLE) static void wb_entropy_get_mutex(void) diff --git a/wolfcrypt/src/wolfentropy.c b/wolfcrypt/src/wolfentropy.c index 3932d652247..692ef267082 100644 --- a/wolfcrypt/src/wolfentropy.c +++ b/wolfcrypt/src/wolfentropy.c @@ -569,12 +569,13 @@ int wc_Entropy_GetRawEntropy(unsigned char* raw, int cnt) if (ret == 0) { ret = Entropy_GetNoise(raw, cnt); } -#ifdef ENTROPY_MEMUSE_THREADED - /* Stop the counter thread to avoid thrashing the system. */ - Entropy_StopThread(); -#endif + /* Only stop the thread and unlock if this thread holds the mutex. */ if (locked) { + #ifdef ENTROPY_MEMUSE_THREADED + /* Stop the counter thread to avoid thrashing the system. */ + Entropy_StopThread(); + #endif wc_UnLockMutex(&entropy_mutex); } @@ -859,6 +860,7 @@ int wc_Entropy_Get(int bits, unsigned char* entropy, word32 len) { int ret = 0; int noise_len; + int locked = 0; static byte noise[MAX_NOISE_CNT]; if (bits <= 0 || bits > MAX_ENTROPY_BITS || (entropy == NULL && len > 0)) { @@ -885,8 +887,13 @@ int wc_Entropy_Get(int bits, unsigned char* entropy, word32 len) #endif /* Lock the mutex as collection uses globals. */ - if ((ret == 0) && (wc_LockMutex(&entropy_mutex) != 0)) { - ret = BAD_MUTEX_E; + if (ret == 0) { + if (wc_LockMutex(&entropy_mutex) != 0) { + ret = BAD_MUTEX_E; + } + else { + locked = 1; + } } #ifdef ENTROPY_MEMUSE_THREADED @@ -939,12 +946,13 @@ int wc_Entropy_Get(int bits, unsigned char* entropy, word32 len) } } -#ifdef ENTROPY_MEMUSE_THREADED - /* Stop the counter thread to avoid thrashing the system. */ - Entropy_StopThread(); -#endif + /* Only stop the thread and unlock if this thread holds the mutex. */ + if (locked) { + #ifdef ENTROPY_MEMUSE_THREADED + /* Stop the counter thread to avoid thrashing the system. */ + Entropy_StopThread(); + #endif - if (ret != WC_NO_ERR_TRACE(BAD_MUTEX_E)) { /* Unlock mutex now we are done. */ wc_UnLockMutex(&entropy_mutex); } @@ -963,18 +971,22 @@ int wc_Entropy_Get(int bits, unsigned char* entropy, word32 len) int wc_Entropy_OnDemandTest(void) { int ret = 0; + int locked = 0; /* Lock the mutex as we don't want collecting to happen during testing. */ if (wc_LockMutex(&entropy_mutex) != 0) { ret = BAD_MUTEX_E; } + else { + locked = 1; + } if (ret == 0) { /* Perform startup tests. */ ret = Entropy_HealthTest_Startup(); } - if (ret != WC_NO_ERR_TRACE(BAD_MUTEX_E)) { + if (locked) { /* Unlock mutex now we are done. */ wc_UnLockMutex(&entropy_mutex); } @@ -989,22 +1001,30 @@ int wc_Entropy_OnDemandTest(void) int Entropy_Init(void) { int ret = 0; + int locked = 0; /* Check whether initialization has succeeded before. */ if (!entropy_memuse_initialized) { #if !defined(SINGLE_THREADED) && !defined(WOLFSSL_MUTEX_INITIALIZER) ret = wc_InitMutex(&entropy_mutex); #endif - if (ret == 0) - ret = wc_LockMutex(&entropy_mutex); + if (ret == 0) { + if (wc_LockMutex(&entropy_mutex) != 0) { + ret = BAD_MUTEX_E; + } + else { + locked = 1; + } + } if (entropy_memuse_initialized) { /* Short circuit return -- a competing thread initialized the state * while we were waiting. Note, this is only threadsafe when * WOLFSSL_MUTEX_INITIALIZER is defined. */ - if (ret == 0) + if (locked) { wc_UnLockMutex(&entropy_mutex); + } return 0; } @@ -1030,7 +1050,7 @@ int Entropy_Init(void) #endif } - if (ret != WC_NO_ERR_TRACE(BAD_MUTEX_E)) { + if (locked) { wc_UnLockMutex(&entropy_mutex); } } From 6d0275a11058604b2a88eb98a2bf4b3af8afe304 Mon Sep 17 00:00:00 2001 From: Emma Stensland Date: Wed, 28 Oct 2026 13:36:55 -0600 Subject: [PATCH 2/3] Fix Entropy_Init() re-init race --- doc/dox_comments/header_files/wolfentropy.h | 24 ++- tests/unit-mcdc/test_wolfentropy_whitebox.c | 45 ++--- wolfcrypt/src/wc_port.c | 11 +- wolfcrypt/src/wolfentropy.c | 189 ++++++++++---------- wolfssl/wolfcrypt/wc_port.h | 2 + 5 files changed, 149 insertions(+), 122 deletions(-) diff --git a/doc/dox_comments/header_files/wolfentropy.h b/doc/dox_comments/header_files/wolfentropy.h index 570ed01e318..c399cba8aa9 100644 --- a/doc/dox_comments/header_files/wolfentropy.h +++ b/doc/dox_comments/header_files/wolfentropy.h @@ -2,9 +2,15 @@ \ingroup wolfEntropy \brief Gets raw entropy without DRBG processing. + For SP 800-90B assessment only, not for seeding. Output is not + conditioned and is not gated on the startup health test, so samples can + be collected from a source that fails it. Creates the mutex and SHA-3 + object on first use. + \return 0 On success - \return BAD_FUNC_ARG If raw is NULL - \return RNG_FAILURE_E Failed + \return BAD_FUNC_ARG If raw is NULL or cnt is not positive + \return BAD_MUTEX_E Unable to lock mutex + \return Negative Other failure (setup, noise collection, thread) \param raw Buffer for entropy \param cnt Bytes to retrieve @@ -24,9 +30,15 @@ int wc_Entropy_GetRawEntropy(unsigned char* raw, int cnt); \ingroup wolfEntropy \brief Gets processed entropy with specified bits. + Creates the mutex and SHA-3 object on first use. Runs the startup + health test once per call while none has passed since the last reset, + and returns its error on failure rather than retrying. + \return 0 On success \return BAD_FUNC_ARG If entropy is NULL - \return RNG_FAILURE_E Failed + \return ENTROPY_RT_E or ENTROPY_APT_E Health test failed + \return BAD_MUTEX_E Unable to lock mutex + \return Negative Other failure \param bits Entropy bits required \param entropy Buffer for entropy @@ -80,8 +92,12 @@ int wc_Entropy_Get(int bits, unsigned char* entropy, word32 len); \ingroup wolfEntropy \brief Tests entropy source on demand. + Creates the mutex and SHA-3 object on first use. + \return 0 On success - \return RNG_FAILURE_E Test failed + \return ENTROPY_RT_E or ENTROPY_APT_E Test failed + \return BAD_MUTEX_E Unable to lock mutex + \return Negative Other failure _Example_ \code diff --git a/tests/unit-mcdc/test_wolfentropy_whitebox.c b/tests/unit-mcdc/test_wolfentropy_whitebox.c index 947818d628b..32eed52f5b7 100644 --- a/tests/unit-mcdc/test_wolfentropy_whitebox.c +++ b/tests/unit-mcdc/test_wolfentropy_whitebox.c @@ -47,14 +47,15 @@ * setup failure is reported as a skip (return 0), never a test failure, so * the harness never discards the variant's coverage. * - * wc_Entropy_Get() itself has two decisions whose operands reference the + * wc_Entropy_Get() has two decisions (the first in Entropy_HealthTest_Passed(), + * shared with Entropy_Init()) whose operands reference the * SAME file-static health-test state, but are not independently selectable * from tests/api because Entropy_Init() (run once by test setup) already * leaves that state primed: * - * "if ((ret == 0) && ((prop_total == 0) || (!rep_have_prev)))" - * - "prop_total == 0" startup-retrigger via the first OR operand - * - "!rep_have_prev" startup-retrigger via the second OR operand + * "return (prop_total != 0) && rep_have_prev;" (Entropy_HealthTest_Passed) + * - "prop_total != 0" startup-retrigger via the first operand + * - "rep_have_prev" startup-retrigger via the second operand * "while ((ret == 0) && (len > 0))" * "for (i = 0; (ret == 0) && (i < noise_len); i++)" * - "ret == 0" both loops only ever end via their *other* @@ -230,13 +231,14 @@ static void wb_collect_path(void) } /* wc_Entropy_Get()'s startup-retrigger guard: - * "if ((ret == 0) && ((prop_total == 0) || (!rep_have_prev)))" + * "if ((ret == 0) && (!Entropy_HealthTest_Passed()))" + * "return (prop_total != 0) && rep_have_prev;" * On any live process, Entropy_Init() (already run by test setup and by * wb_collect_path() above) leaves prop_total != 0 and rep_have_prev == 1, so - * this guard's *true* side -- and each of its two OR operands individually + * this guard's *true* side -- and each of its two operands individually * -- is never shown from tests/api. Both globals are file-static: drive * each operand's independence pair directly, holding the other operand - * false (the "both false" side is already exercised by every other + * true (the "both true" side is already exercised by every other * steady-state call in this suite, e.g. wb_collect_path() above). */ static void wb_startup_retrigger(void) { @@ -249,8 +251,8 @@ static void wb_startup_retrigger(void) return; } - /* "prop_total == 0" true, "!rep_have_prev" false: retrigger via the - * first OR operand alone. */ + /* "prop_total != 0" false, "rep_have_prev" true: retrigger via the + * first operand alone. */ prop_total = 0; rep_have_prev = 1; XMEMSET(out, 0, sizeof(out)); @@ -260,8 +262,8 @@ static void wb_startup_retrigger(void) wb_fail = 1; } - /* "prop_total == 0" false, "!rep_have_prev" true: retrigger via the - * second OR operand alone. */ + /* "prop_total != 0" true, "rep_have_prev" false: retrigger via the + * second operand alone. */ prop_total = 1; rep_have_prev = 0; XMEMSET(out, 0, sizeof(out)); @@ -392,24 +394,25 @@ static void wb_startup_noise_fail(void) /* ---- wc_Entropy_Get() mutex-failure vectors ------------------------------ * * - * LOCK: if (wc_LockMutex(&entropy_mutex) != 0) (inside "if (ret == 0)") - * PROP: if ((ret == 0) && ((prop_total == 0) || (!rep_have_prev))) + * LOCK: if (wc_LockMutex(&entropy_mutex) != 0) (Entropy_SetupAndLock) + * PROP: if ((ret == 0) && (!Entropy_HealthTest_Passed())) * * A live, correctly initialised mutex always locks, so LOCK only ever shows * FALSE and PROP only ever shows its idx0 TRUE half. mcdc_fault_mutex.h * redirects this TU's wc_LockMutex() through a hook; mcdc_fm_lock_once makes * the NEXT lock -- and only that one -- refuse: * - * armed -> LOCK TRUE -> ret = BAD_MUTEX_E, which then makes PROP's idx0 - * FALSE at the very next decision (same call, same binary) + * armed -> LOCK TRUE -> ret = BAD_MUTEX_E, which also makes idx0 of + * Entropy_SetupAndLock()'s SHA-3 setup guard FALSE and later + * PROP's idx0 FALSE (same call, same binary) * unarmed -> LOCK FALSE and PROP idx0 TRUE * * A refused lock is the one path wc_Entropy_Get() must NOT unlock on, and it * doesn't: the tail is guarded on `locked`, so no unlock of an - * unheld mutex happens and no global state is touched. The enclosing - * "ret == 0" guard's FALSE half stays a justified residual: outside HAVE_FIPS - * builds nothing runs between "ret = 0" and this test, so ret is 0 by - * construction. + * unheld mutex happens and no global state is touched. In builds without a + * static mutex initializer, the "ret == 0" guard around LOCK has its FALSE + * half unexercised: it needs wc_local_InitMutexOnce() to fail, which is not + * hooked. * ------------------------------------------------------------------------ */ #if defined(HAVE_ENTROPY_MEMUSE) && !defined(MCDC_FM_UNAVAILABLE) static void wb_entropy_get_mutex(void) @@ -419,8 +422,8 @@ static void wb_entropy_get_mutex(void) XMEMSET(out, 0, sizeof(out)); - /* Warm-up while unarmed so lazy initialisation (which locks the same - * mutex) is done before the one-shot fault is armed. */ + /* Warm-up while unarmed to set up SHA-3 and health test state again + * after the earlier Entropy_Final(). */ (void)wc_Entropy_Get(MAX_ENTROPY_BITS, out, (word32)sizeof(out)); /* Armed: the collection lock is refused exactly once. */ diff --git a/wolfcrypt/src/wc_port.c b/wolfcrypt/src/wc_port.c index 20263890bc1..aa55191735a 100644 --- a/wolfcrypt/src/wc_port.c +++ b/wolfcrypt/src/wc_port.c @@ -3081,7 +3081,7 @@ int wc_local_InitMutexOnce(wolfSSL_Mutex* m, wc_MutexOnceFlag* flag) #if defined(WOLFSSL_ATOMIC_OPS) && !defined(SINGLE_THREADED) WC_ATOMIC_UINT_ARG expected; - if (WOLFSSL_ATOMIC_LOAD(*flag) == 2) { + if (WOLFSSL_ATOMIC_LOAD(*flag) == WC_MUTEX_READY_ONCE) { return 0; } @@ -3090,10 +3090,11 @@ int wc_local_InitMutexOnce(wolfSSL_Mutex* m, wc_MutexOnceFlag* flag) if (wolfSSL_Atomic_Uint_CompareExchange(flag, &expected, 1) == 1) { /* Won the race. On failure reset to 0 so a later call retries. */ ret = wc_InitMutex(m); - WOLFSSL_ATOMIC_STORE(*flag, (ret == 0) ? 2U : 0U); + WOLFSSL_ATOMIC_STORE(*flag, + (ret == 0) ? WC_MUTEX_READY_ONCE : 0U); break; } - if (expected == 2) { + if (expected == WC_MUTEX_READY_ONCE) { /* Another thread completed the initialization. */ break; } @@ -3103,10 +3104,10 @@ int wc_local_InitMutexOnce(wolfSSL_Mutex* m, wc_MutexOnceFlag* flag) #else /* No atomics to elect with: single-threaded cannot race, and opt-out * builds keep their existing behavior. */ - if (*flag != 2) { + if (*flag != WC_MUTEX_READY_ONCE) { ret = wc_InitMutex(m); if (ret == 0) { - *flag = 2; + *flag = WC_MUTEX_READY_ONCE; } } #endif diff --git a/wolfcrypt/src/wolfentropy.c b/wolfcrypt/src/wolfentropy.c index 692ef267082..3790d06b972 100644 --- a/wolfcrypt/src/wolfentropy.c +++ b/wolfcrypt/src/wolfentropy.c @@ -66,7 +66,10 @@ data, use this implementation to seed and re-seed the DRBG. /* Maximum number of bytes to sample to produce max entropy. */ #define MAX_NOISE_CNT (MAX_ENTROPY_BITS * 8 + ENTROPY_EXTRA) -/* MemUse entropy global state initialized. */ +/* MemUse entropy mutex and SHA-3 object ready. Health test status is kept + * separately. Only accessed while holding entropy_mutex, except by + * Entropy_Final(), which must not run concurrently with other wolfEntropy + * calls (only called from wolfCrypt_Cleanup()). */ static volatile int entropy_memuse_initialized = 0; /* Global SHA-3 object used for conditioning entropy and creating noise. */ static wc_Sha3 entropyHash; @@ -526,14 +529,57 @@ static int Entropy_GetNoise(unsigned char* noise, int samples) * same time. */ static wolfSSL_Mutex entropy_mutex WOLFSSL_MUTEX_INITIALIZER_CLAUSE(entropy_mutex); +#if !defined(SINGLE_THREADED) && !defined(WOLFSSL_MUTEX_INITIALIZER) +static wc_MutexOnceFlag entropy_mutex_init = WOLFSSL_ATOMIC_INITIALIZER(0); +#endif + +/* Lock the mutex, creating it and the SHA-3 object on first use. Does not run + * the startup health test, so raw noise can be collected from a source that + * fails it. + * + * @param [out] locked 1 when this thread now holds the mutex, else 0. + * @return 0 on success. + * @return BAD_MUTEX_E when the mutex cannot be locked. + * @return Negative when setup fails. + */ +static int Entropy_SetupAndLock(int* locked) +{ + int ret = 0; + + *locked = 0; +#if !defined(SINGLE_THREADED) && !defined(WOLFSSL_MUTEX_INITIALIZER) + ret = wc_local_InitMutexOnce(&entropy_mutex, &entropy_mutex_init); + if (ret == 0) +#endif + { + if (wc_LockMutex(&entropy_mutex) != 0) { + ret = BAD_MUTEX_E; + } + else { + *locked = 1; + } + } + + if ((ret == 0) && (!entropy_memuse_initialized)) { + ret = wc_InitSha3_256(&entropyHash, NULL, INVALID_DEVID); + if (ret == 0) { + entropy_memuse_initialized = 1; + } + } + + return ret; +} /* Generate raw entropy for performing assessment. + * + * Not gated on the startup health test so a failing source can be assessed. * * @param [out] raw Buffer to hold raw entropy data. * @param [in] cnt Number of bytes of raw entropy to get. * @return 0 on success. - * @return Negative when creating a thread fails - when no high resolution - * clock available. + * @return BAD_MUTEX_E when the mutex cannot be locked. + * @return Negative when setup or noise collection fails, or when creating a + * thread fails - when no high resolution clock available. */ int wc_Entropy_GetRawEntropy(unsigned char* raw, int cnt) { @@ -544,21 +590,8 @@ int wc_Entropy_GetRawEntropy(unsigned char* raw, int cnt) return BAD_FUNC_ARG; } -#ifdef HAVE_FIPS - if (!entropy_memuse_initialized) { - ret = Entropy_Init(); - } -#endif - /* Lock the mutex as collection uses globals. */ - if (ret == 0) { - if (wc_LockMutex(&entropy_mutex) != 0) { - ret = BAD_MUTEX_E; - } - else { - locked = 1; - } - } + ret = Entropy_SetupAndLock(&locked); #ifdef ENTROPY_MEMUSE_THREADED if (ret == 0) { @@ -801,6 +834,18 @@ static int Entropy_HealthTest_Startup(void) return ret; } +/* Check whether a startup health test has passed since the last reset. + * + * Caller must hold entropy_mutex. + * + * @return 1 when passed. + * @return 0 otherwise. + */ +static int Entropy_HealthTest_Passed(void) +{ + return (prop_total != 0) && rep_have_prev; +} + /* Condition raw entropy noise using SHA-3-256. * * Put noise into a hash function: SHA-3-256. @@ -855,6 +900,7 @@ static int Entropy_Condition(byte* output, word32 len, byte* noise, * @return 0 on success. * @return ENTROPY_RT_E or ENTROPY_APT_E on failure. * @return BAD_MUTEX_E when unable to lock mutex. + * @return Negative on other failure. */ int wc_Entropy_Get(int bits, unsigned char* entropy, word32 len) { @@ -871,30 +917,17 @@ int wc_Entropy_Get(int bits, unsigned char* entropy, word32 len) * entropy requested. */ noise_len = (bits + ENTROPY_EXTRA) / ENTROPY_MIN; -#ifdef HAVE_FIPS /* FIPS KATs, e.g. EccPrimitiveZ_KnownAnswerTest(), call wc_Entropy_Get() * incidental to wc_InitRng(), without first calling Entropy_Init(), neither * directly, nor indirectly via wolfCrypt_Init(). This matters, because * KATs must be usable before wolfCrypt_Init() (indeed, in the library * embodiment, the HMAC KAT always runs before wolfCrypt_Init(), incidental - * to fipsEntry()). Without the InitSha3() under Entropy_Init(), the - * SHA3_BLOCK function pointer is null when Sha3Update() is called by - * Entropy_MemUse(), which ends badly. + * to fipsEntry()). Without the InitSha3() under Entropy_SetupAndLock(), + * the SHA3_BLOCK function pointer is null when Sha3Update() is called by + * Entropy_MemUse(), which ends badly. Also retries a failed setup in all + * builds. The startup health test runs below, under the mutex. */ - if (!entropy_memuse_initialized) { - ret = Entropy_Init(); - } -#endif - - /* Lock the mutex as collection uses globals. */ - if (ret == 0) { - if (wc_LockMutex(&entropy_mutex) != 0) { - ret = BAD_MUTEX_E; - } - else { - locked = 1; - } - } + ret = Entropy_SetupAndLock(&locked); #ifdef ENTROPY_MEMUSE_THREADED if (ret == 0) { @@ -904,7 +937,7 @@ int wc_Entropy_Get(int bits, unsigned char* entropy, word32 len) #endif /* Check we have had a startup health check pass. */ - if ((ret == 0) && ((prop_total == 0) || (!rep_have_prev))) { + if ((ret == 0) && (!Entropy_HealthTest_Passed())) { /* Try again as check failed. */ ret = Entropy_HealthTest_Startup(); } @@ -967,6 +1000,7 @@ int wc_Entropy_Get(int bits, unsigned char* entropy, word32 len) * @return 0 on success. * @return ENTROPY_RT_E or ENTROPY_APT_E on failure. * @return BAD_MUTEX_E when unable to lock mutex. + * @return Negative on other failure. */ int wc_Entropy_OnDemandTest(void) { @@ -974,12 +1008,7 @@ int wc_Entropy_OnDemandTest(void) int locked = 0; /* Lock the mutex as we don't want collecting to happen during testing. */ - if (wc_LockMutex(&entropy_mutex) != 0) { - ret = BAD_MUTEX_E; - } - else { - locked = 1; - } + ret = Entropy_SetupAndLock(&locked); if (ret == 0) { /* Perform startup tests. */ @@ -994,6 +1023,8 @@ int wc_Entropy_OnDemandTest(void) } /* Initialize global state for MemUse Entropy and do startup health test. + * + * Reruns the startup health test on each call until one passes. * * @return 0 on success. * @return Negative on failure. @@ -1003,56 +1034,26 @@ int Entropy_Init(void) int ret = 0; int locked = 0; - /* Check whether initialization has succeeded before. */ - if (!entropy_memuse_initialized) { - #if !defined(SINGLE_THREADED) && !defined(WOLFSSL_MUTEX_INITIALIZER) - ret = wc_InitMutex(&entropy_mutex); - #endif - if (ret == 0) { - if (wc_LockMutex(&entropy_mutex) != 0) { - ret = BAD_MUTEX_E; - } - else { - locked = 1; - } - } + ret = Entropy_SetupAndLock(&locked); - if (entropy_memuse_initialized) { - /* Short circuit return -- a competing thread initialized the state - * while we were waiting. Note, this is only threadsafe when - * WOLFSSL_MUTEX_INITIALIZER is defined. - */ - if (locked) { - wc_UnLockMutex(&entropy_mutex); - } - return 0; - } - - if (ret == 0) { - /* Initialize a SHA3-256 object for use in entropy operations. */ - ret = wc_InitSha3_256(&entropyHash, NULL, INVALID_DEVID); - } - /* Set globals initialized. */ - entropy_memuse_initialized = (ret == 0); - if (ret == 0) { - #ifdef ENTROPY_MEMUSE_THREADED - /* Start the counter thread as a proxy for time counter. */ - ret = Entropy_StartThread(); - if (ret == 0) - #endif - { - /* Do first startup test now. */ - ret = Entropy_HealthTest_Startup(); - } - #ifdef ENTROPY_MEMUSE_THREADED - /* Stop the counter thread to avoid thrashing the system. */ - Entropy_StopThread(); - #endif + /* Run the startup health test unless one has already passed. */ + if ((ret == 0) && (!Entropy_HealthTest_Passed())) { + #ifdef ENTROPY_MEMUSE_THREADED + /* Start the counter thread as a proxy for time counter. */ + ret = Entropy_StartThread(); + if (ret == 0) + #endif + { + ret = Entropy_HealthTest_Startup(); } + #ifdef ENTROPY_MEMUSE_THREADED + /* Stop the counter thread to avoid thrashing the system. */ + Entropy_StopThread(); + #endif + } - if (locked) { - wc_UnLockMutex(&entropy_mutex); - } + if (locked) { + wc_UnLockMutex(&entropy_mutex); } return ret; @@ -1066,14 +1067,18 @@ void Entropy_Final(void) if (entropy_memuse_initialized) { /* Dispose of the SHA3-356 hash object. */ wc_Sha3_256_Free(&entropyHash); - #if !defined(SINGLE_THREADED) && !defined(WOLFSSL_MUTEX_INITIALIZER) - wc_FreeMutex(&entropy_mutex); - #endif /* Clear health test data. */ Entropy_HealthTest_Reset(); /* No longer initialized. */ entropy_memuse_initialized = 0; } +#if !defined(SINGLE_THREADED) && !defined(WOLFSSL_MUTEX_INITIALIZER) + /* Free the mutex if created, even if SHA-3 setup failed. */ + if (WOLFSSL_ATOMIC_LOAD(entropy_mutex_init) == WC_MUTEX_READY_ONCE) { + wc_FreeMutex(&entropy_mutex); + WOLFSSL_ATOMIC_STORE(entropy_mutex_init, 0); + } +#endif } /* Reset the data associated with the MemUse Entropy health tests. diff --git a/wolfssl/wolfcrypt/wc_port.h b/wolfssl/wolfcrypt/wc_port.h index d61ee49f6cb..8f33290e6b0 100644 --- a/wolfssl/wolfcrypt/wc_port.h +++ b/wolfssl/wolfcrypt/wc_port.h @@ -1202,6 +1202,8 @@ WOLFSSL_LOCAL int wc_CancelDisable(void); /* returns the old state */ WOLFSSL_LOCAL void wc_CancelRestore(int state); WOLFSSL_API wolfSSL_Mutex* wc_InitAndAllocMutex(void); #ifndef WOLFSSL_MUTEX_INITIALIZER + /* wc_MutexOnceFlag value once the mutex is created. */ + #define WC_MUTEX_READY_ONCE 2U /* Election state for wc_local_InitMutexOnce(). Define objects with * WOLFSSL_ATOMIC_INITIALIZER(0) and only touch them through * WOLFSSL_ATOMIC_LOAD/STORE. */ From 47ab7a64f5819b635f70aee47e4685ae6644f20e Mon Sep 17 00:00:00 2001 From: Emma Stensland Date: Wed, 28 Oct 2026 16:46:53 -0600 Subject: [PATCH 3/3] Start counter thread in wc_Entropy_OnDemandTest() --- wolfcrypt/src/wolfentropy.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/wolfcrypt/src/wolfentropy.c b/wolfcrypt/src/wolfentropy.c index 3790d06b972..64f84c69fe5 100644 --- a/wolfcrypt/src/wolfentropy.c +++ b/wolfcrypt/src/wolfentropy.c @@ -1010,12 +1010,23 @@ int wc_Entropy_OnDemandTest(void) /* Lock the mutex as we don't want collecting to happen during testing. */ ret = Entropy_SetupAndLock(&locked); +#ifdef ENTROPY_MEMUSE_THREADED + if (ret == 0) { + /* Start the counter thread as a proxy for time counter. */ + ret = Entropy_StartThread(); + } +#endif if (ret == 0) { /* Perform startup tests. */ ret = Entropy_HealthTest_Startup(); } + /* Only stop the thread and unlock if this thread holds the mutex. */ if (locked) { + #ifdef ENTROPY_MEMUSE_THREADED + /* Stop the counter thread to avoid thrashing the system. */ + Entropy_StopThread(); + #endif /* Unlock mutex now we are done. */ wc_UnLockMutex(&entropy_mutex); }