20260920-rng-fixes - #11510
20260920-rng-fixes#11510
Conversation
|
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11510
Scan targets checked: linuxkm-src, linuxkm-bugs, wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Findings: 6
6 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
|
Retest this please. |
c90c373 to
ac9c842
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 1
Open (11)
On !CONFIG_PREEMPT_RT, enabling preemption while holding aspinlock_tcan deadlock (the lock… · New Usingvolatilefor cross-context synchronization in kernel code is unreliable and discouraged.… · New Usingvolatilefor cross-context synchronization in kernel code is unreliable and discouraged.… · New Usingvolatilefor cross-context synchronization in kernel code is unreliable and discouraged.… · Newmalloc()/free()are not standard kernel allocation APIs and (unless locally remapped) may not… · Newmalloc()/free()are not standard kernel allocation APIs and (unless locally remapped) may not… · Newmalloc()/free()are not standard kernel allocation APIs and (unless locally remapped) may not… · New These wipes truncatelen(asize_t) toword32. If callers can request more thanUINT32_MAX… · New These wipes truncatelen(asize_t) toword32. If callers can request more thanUINT32_MAX… · Newwc_RNG_DRBG_GetReseedCtr()return value is ignored, butctris still used to decide… · New This mandatory next-seed redemption path assignsretbut only exits early forDRBG_CONT_FIPS_E.… · New
What changed in this PR
This PR tightens DRBG failure semantics (failure-atomic reseed/generate with no state mutation on error), extends hash APIs with reusable Reset() entrypoints, and improves linuxkm RNG recovery/error-trace behavior.
Changes:
- Add
Reset()APIs for SHA-256/SHA-224, SHA-512/SHA-384 and SHA-3/SHAKE, plus associated unit tests and doxygen docs. - Refactor Hash_DRBG reseed/generate to be failure-atomic; adjust scratch sizing and move “next-stir” storage into
WC_RNG. - linuxkm: add RNG invalidation recovery window handling, revise module mutex behavior, and fix errno-domain comparisons with
WC_NO_ERR_TRACE().
| File | Description |
|---|---|
| wolfssl/wolfcrypt/sha512.h | Add public wc_Sha512*Reset() / wc_Sha384Reset() declarations. |
| wolfssl/wolfcrypt/sha3.h | Add public wc_Sha3_*_Reset() / wc_Shake*_Reset() declarations. |
| wolfssl/wolfcrypt/sha256.h | Add public wc_Sha256Reset() / wc_Sha224Reset() declarations. |
| wolfssl/wolfcrypt/rng_bank.h | Introduce root RNG / inst-op gate feature macros; add root_rng_reinit() API. |
| wolfssl/wolfcrypt/random.h | Move next-stir buffer to WC_RNG; enlarge small-stack-cache scratch buffers. |
| wolfcrypt/test/test.c | Add reset-equivalence tests; adjust SVR gating; update next-stir length macro usage. |
| wolfcrypt/src/sha512.c | Implement SHA-512/SHA-384 reset-in-place plus fallback reset implementations. |
| wolfcrypt/src/sha3.c | Implement reset helper and per-algorithm reset wrappers for SHA-3/SHAKE. |
| wolfcrypt/src/sha256.c | Implement SHA-256/SHA-224 reset-in-place plus fallback reset implementations. |
| wolfcrypt/src/rng_bank.c | Add inst-op gate spinenter; root RNG reinit; expand gate coverage to invalidation walk. |
| wolfcrypt/src/random.c | Make DRBG reseed/generate failure-atomic; revise next-stir plumbing; improve reseed-now failure handling. |
| tests/unit-mcdc/test_sp_int_whitebox.c | Update real-entropy sweep comment to match new failure-atomic contract. |
| tests/unit-mcdc/test_random_whitebox.c | Update Hash_DRBG_Generate / Hash512_DRBG_Generate call signatures in tests. |
| tests/unit-mcdc/test_random_fault_whitebox.c | Add fault vectors asserting reseed/generate failure-atomicity; fix “done(with failures)” output. |
| tests/api/test_random.c | Update HealthTest bad-argument expectations under specific FIPS/selftest builds. |
| linuxkm/module_hooks.c | Revise error-trace enums; change lock behavior; add wc_lkm_UnlockMutex; fix errno comparisons. |
| linuxkm/lkcapi_sha_glue.c | Add RNG invalidation recovery window checks; root_rng recovery/reinit; retry loops; buffer wiping. |
| linuxkm/lkcapi_rsa_glue.c | Fix PTR_ERR() comparisons to use -WC_NO_ERR_TRACE(ENOMEM). |
| linuxkm/lkcapi_ecdsa_glue.c | Fix errno comparisons to use -WC_NO_ERR_TRACE(...). |
| linuxkm/lkcapi_ecdh_glue.c | Fix PTR_ERR() comparisons to use -WC_NO_ERR_TRACE(ENOMEM). |
| linuxkm/lkcapi_dh_glue.c | Fix PTR_ERR() comparisons to use -WC_NO_ERR_TRACE(ENOMEM). |
| linuxkm/linuxkm_wc_port.h | Add unlock symbol indirection; revise mutex state field; add WC_SVR_HAVE_FLAGS. |
| doc/dox_comments/header_files/sha512.h | Document new SHA-512/SHA-384 reset APIs. |
| doc/dox_comments/header_files/sha3.h | Document new SHA-3/SHAKE reset APIs. |
| doc/dox_comments/header_files/sha256.h | Document new SHA-256/SHA-224 reset APIs. |
| doc/dox_comments/header_files/rng_bank.h | Document new wc_rng_bank_root_rng_reinit() API. |
| doc/dox_comments/header_files/random.h | Update docs to WC_RNG_NEXT_STIR_LEN. |
| .wolfssl_known_macro_extras | Add WC_LINUXKM_SPIN_IN_ATOMIC; reorder macro entry. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11510
Scan targets checked: linuxkm-src, linuxkm-bugs, wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
ac9c842 to
e86aaed
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11510
Scan targets checked: linuxkm-src, linuxkm-bugs, wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Findings: 6
6 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
…est_random_whitebox.c:
Make Hash*_DRBG() failure-atomic.
Eliminate the mutation seams in Hash_DRBG_Reseed/Hash512 and
Hash_DRBG_Generate/Hash512: reseed derives newV and newC into scratch and
commits both only after both Hash_df calls succeed; generate runs the SP 800-90A
10.1.1.4 step-2 fold ("additional") and all subsequent hashing against a shadow
V, committing V/C/reseedCtr only after the last fallible step. With these
refinements, a failed operation is now externally and internally
indistinguishable from one never attempted, eliminating the recently introduced
state_mutated condemnation machinery (and the WC_RNG* argument it required).
New contract, stated at both function pairs: no state mutation on failure.
(Note that fault-injection coverage of the new contract -- no-mutation vectors
and byte-identical-retry witnesses in tests/unit-mcdc/ -- lands in the following
commit.)
Condemnation survives as-was at instantiation failure, stranded-lock exits,
continuous-test failure, and all forced reseed paths.
The new wc_RNG_DRBG_Reseed_Now() and wc_RNG_DRBG_NextSeedNow*() are fixed to
check the DRBG reseed counter on failure, and only condemn the DRBG if the
reseed was forced as in wc_RNG_GenerateBlock(), otherwise the DRBG is healthy
and can continue. Reseed failures are reported to the caller regardless of
condemnation.
…_int_whitebox.c: Add DRBG failure-atomicity fault vectors and re-measure the sp_int real-entropy sweep, for random.c's no-mutation-on-failure contract. (Coverage companion to the preceding commit's failure-atomicity refactor of random.c, landed separately so the contract change and its executable referee are independently reviewable; no test in the tree asserted the superseded behavior, so the intermediate state is green throughout.) In test_random_fault_whitebox.c, add wb_hash_drbg_reseed_df_chain() / wb_hash512_drbg_reseed_df_chain() and wb_hash_drbg_generate_atomicity() / wb_hash512_drbg_generate_atomicity(). The reseed vectors fault each of Hash256_DRBG_Reseed()'s / Hash512_DRBG_Reseed()'s two derivation calls ((F,-) and (T,F)) and assert DRBG_FAILURE with V, C, and reseedCtr byte-identical, then retry the same reseed disarmed and require clean success; vector (T,F) is the old seam's signature, where the pre-atomic code had already committed V before deriving C. The generate vectors fault the first primitive before Hash_gen() (the 0x02 additional-input hash) and the first primitive after it (the 0x03 V-update hash, with the output block already produced) and assert no state mutation, then require the disarmed retry to produce output byte-identical to an identically-instantiated control and to land in the control's exact end state -- a failed generate consumes no stream position, and the identical retry bytes are the executable witness that Hash_gen() banks nothing across a failed attempt. Arm indices are measured via mcdc_fh_seen(), never hard-coded. Detection power was validated by temporarily reintroducing the old commit order in the SHA-256 cores (V committed between the reseed's derivations, thisV committed before the 0x03 hash): exactly the predicted assertions fired -- reseed (T,F) mutation, post-Hash_gen mutation, retry divergence -- and nothing else. Also change the TU's failure report from "done (with skips)" to "done (with failures)", the convention the rest of the suite uses and the string run-whitebox-smoke.sh greps for, so this TU's assertions (new and existing) are enforced by the smoke gate. smoke-expected.txt is unchanged: the TU was already listed and still passes. In test_sp_int_whitebox.c, rewrite the real-entropy sweep comment in wb_prime_trial_alloc(), which recorded condemnation-era behavior (first faulted allocation inside Hash_DRBG_Generate() left the WC_RNG permanently DRBG_FAILED, every later index died at the draw, :19672:0 unreachable from this sweep). Re-measured under WOLFSSL_SMALL_STACK: n = 1..2 fault the function's own ALLOC_SP_INT_ARRAYs (MP_MEM); n = 3..5 fault the draw's three allocations inside Hash_DRBG_Generate() and return RNG_FAILURE_E for that index only, with rng.status remaining WC_DRBG_OK throughout; n >= 6 lands inside the exponentiation and breaks the trial loop with MP_MEM, so the sweep now reaches :19672:0 as well. The pinned-RNG block remains the deterministic closer of record for that key. Comment-only change; the sweep asserts nothing. Verified with --enable-all-crypto --enable-cryptonly --enable-static, plus --enable-smallstack and --enable-smallstack --enable-smallstackcache variants: wolfcrypt/test/testwolfcrypt and the random/sp_int white-box TUs pass under all three, and run-whitebox-smoke.sh reports no failures and no expected-list changes.
…e fixes.
Fix error-trace machinery and add missing WC_NO_ERR_TRACE()s: errno-domain
comparisons in the PK and SHA glue and module_hooks (ENOMEM, EAGAIN, EINTR,
EBADMSG, and wolfCrypt INTERRUPTED_E/NOT_READY_E sites) now use the
-WC_NO_ERR_TRACE(E*) idiom, with .wolfssl_known_macro_extras updated to match.
Change default kernel "mutex" (spinlock) behavior: atomic-context callers now
never spin -- they trylock and fail fast -- and task-context lockers re-enable
preemption (and with it, migration) immediately after acquisition, rebalancing
the preempt count just before release. The legacy behavior -- task-context
holders promoted to non-preemptible for the duration of the hold, permitting
atomic-context callers (except NMI) to spin-wait -- remains available via a new
WC_LINUXKM_SPIN_IN_ATOMIC macro, which is #error-incompatible with
CONFIG_PREEMPT_RT; the mutex state field (.irq_flags vs .preempt_reenabled)
follows the selected regime, and wc_lkm_UnlockMutex() moves out of line (with a
pie redirect table entry).
Add pre- and post-checks bracketing DRBG operations against the invalidation
recovery window (keyed off last_invalidation_at, stamped at invalidation). The
pre-check works as a waiting room, keeping callers from generating a CAS scan
storm during the invalidation recovery interval (during which there are no
usable RNGs system-wide). In the post-check, -EAGAIN passes through for another
lap, a window overrun returns -ETIMEDOUT, atomic-context callers get one-shot
-EBUSY (blockability is checked before signal state, because in hardirq context
`current` is an unrelated task), pending signals return -EINTR, and otherwise
the caller relaxes and retries. Both checks have stubs so reduced
configurations build, and in-window messaging is ratelimited.
Add emergency reinitialization of the entropy daemon's root_rng: when the
recovery sweep finds the root invalidated, the daemon retries
wc_RNG_DRBG_Reseed_Now() with an uncredited timestamp nonce while the failure
remains retryable (BUSY_E, NOT_READY_E, ENTROPY_RT_E, ENTROPY_APT_E), relaxing
between attempts; for the default bank the reseed runs with preemption disabled
to maximize its chances. If the reseed fails terminally, the daemon makes one
last-ditch recovery via wc_FreeRng() and wc_rng_bank_root_rng_init() (or
wc_InitRng() for a standalone root); a successful reinit always clears the
invalidated state. If that too fails, the daemon exits, leaving bank instances
to self-reseed. Outcomes are reported at severities differentiated by default
vs non-default bank (pr_notice/pr_info success, pr_emerg/pr_err failure). The
daemon also now supports a standalone malloc'd root_rng where no bank features
are built, instantiated at startup and freed on exit.
Also:
* wc_linuxkm_drbg_generate_tfm(): retry loop around the invalidation window
replaces the single-shot call; output buffer is zeroized on terminal error.
* wc__get_random_bytes(): honors the recovery window, and zeroizes its
staging buffer both before retry re-entry and on the error exit. An
invalidation may be peculiar to this VM and leave the bytes live in another
VM, so they are wiped when abandoned, as possible CSP elsewhere.
* wc_linuxkm_entropy_daemon(): the registered-leaf recovery pass is now
additionally gated on WC_LINUXKM_HAVE_RNG_REGISTRY, matching its actual
dependency; seed health test failures (ENTROPY_RT_E/ENTROPY_APT_E) on bank
instances are now reported unconditionally at pr_warn_ratelimited severity
("entropy source suspect"), rather than pr_err only under WC_VERBOSE_RNG;
the WC_RNG_DEBUG_STATS dump at daemon exit is removed (was redundant).
* wolfcrypt/test/test.c: SVR access-counter checks now additionally gated on
new WC_SVR_HAVE_FLAGS, matching the configurations that actually maintain
the counters.
restarted like the Final APIs but with no output. Plain-software configs reset in place with no heap calls; other configs fall back to Free + Init_ex, preserving heap hint and devId. * wc_Sha256Reset() * wc_Sha224Reset() * wc_Sha512Reset() * wc_Sha512_224Reset() * wc_Sha512_256Reset() * wc_Sha384Reset() * wc_Sha3_224_Reset() * wc_Sha3_256_Reset() * wc_Sha3_384_Reset() * wc_Sha3_512_Reset() * wc_Shake128_Reset() * wc_Shake256_Reset()
…header_files/random.h, wolfcrypt/src/rng_bank.c, wolfssl/wolfcrypt/rng_bank.h, wolfcrypt/test/test.c, tests/api/test_random.c:
Fixes for deallocation races, unpercolated errors, and gating flubs in
random.c/rng_bank.c:
* Move the NextStir aperture into WC_RNG;
* Add must-succeed DRBG health checks to new reseed APIs;
* Fix the DRBG health test error codes for consistency and specificity.
* Fixes for previously unrecovered internal SHA failures in low level DRBG
implementations (WOLFSSL_SMALL_STACK_CACHE builds only).
* Add WC_RNG_BANK_HAVE_ROOT_RNG, and consistently pivot on it throughout
wolfssl/wolfcrypt/rng_bank.h and wolfcrypt/src/rng_bank.c.
Details:
* Relocate nextStir/nextStirLen from DRBG_internal/DRBG_SHA512_internal to
WC_RNG itself, fixing a deallocation race in wc_RNG_DRBG_NextStirStore()
* The stir is generic, not DRBG-specific, making this refactor natural.
* Splits stir functionality out of wc_RNG_DRBG_NextSeedNow_Nonce_local() into
wc_RNG_DRBG_NextStirNow_local(), streamlining both.
* Pending stir material now survives DRBG reinstantiation and
condemnation-recovery reinit -- this fixes the deallocation race in
wc_RNG_DRBG_NextStirStore().
* WC_DRBG_NEXT_STIR_LEN renamed to WC_RNG_NEXT_STIR_LEN to reflect flavor independence.
* Health-test error codes:
* DRBG_CONT_FAILURE now means exactly one thing -- a ConstantCompare mismatch
in wc_RNG_HealthTestLocal(), its sole producer, signifying unrecoverable
back end cryptographic failure.
* The _ex_internal helpers return DRBG_FAILURE or BAD_FUNC_ARG rather than
blanket WC_FAILURE (generic -1).
* PollAndReSeed() propagates instead of collapsing codes together, with
percolation of distinct root causes (MEMORY_E, ENTROPY_RT_E, ENTROPY_APT_E,
seed-source errors), while positive protocol-domain codes (DRBG_FAILURE,
DRBG_CONT_FAILURE, etc.) are translated at the public boundaries.
* rng->status carries the disposition, while return carries the cause.
* Note that wc_RNG_HealthTest(_ex) with a wrong-size output now returns
BAD_FUNC_ARG, whereas previously it returned generic -1.
* Reseed error handling in wc_RNG_DRBG_Reseed_Now() and
wc_RNG_DRBG_NextSeedNow_Nonce_local():
* Fixed to force rng->status to DRBG_CONT_FAILED on KAT mismatch
(deterministic, non-retryable), returning DRBG_CONT_FIPS_E.
* Seed source failure fixed for consistency with wc_RNG_GenerateBlock(): force
rng->status to DRBG_FAILED when the reseed counter is exhausted. The seed
failure code is returned verbatim unless it is an internal code
(DRBG_FAILURE, DRBG_NO_SEED_CB, etc.), in which case RNG_FAILURE_E or
NOT_READY_E is returned, depending on whether the RNG was condemned for
failed mandatory reseed.
* Fix failure paths in Hash_df/Hash_gen/Hash_DRBG_Generate (both DRBG flavors)
in WOLFSSL_SMALL_STACK_CACHE builds:
* Unconditionally reset the cached hash context via
wc_Sha256Reset()/wc_Sha512Reset(), so a failed operation cannot leave a
partially updated context for the next use.
* Back story: wc_Sha256Final()/wc_Sha512Final() are only reachable if previous
SHA calls all succeed; on midstream failure, the existing code left the SHA
object in an indeterminate state.
* WC_RNG_BANK_HAVE_ROOT_RNG is added as a consistent marker that struct
wc_rng_bank has a root_rng member. The new macro is used to assure proper
initialization and teardown along with the bank.
Fixes for RNG invalidation handling, WC_RNG_BANK_HAVE_ROOT_RNG gating, and CONFIG_PREEMPT_RT preemption balance: * Pivot on WC_RNG_BANK_HAVE_ROOT_RNG for the entropy daemon's root_rng setup/teardown, the root stats dump, and the invalidation pre/post checks, replacing hand-spelled feature disjunctions -- glue now follows root_rng member existence exactly, including the WC_RNG_HAVE_LOCK-only configs the old spellings missed, and root-less configs now compile the affected functions cleanly. * Wipe staged output on invalidation in the user-facing paths: wc_get_random_bytes_user() and wc_extract_crng_user() now ForceZero the staged block on EAGAIN before the post-check. Bytes generated before an invalidation was noticed may be live in a forked VM; wc__get_random_bytes() already wiped, the user paths did not. * Rework invalidation retry in wc__get_random_bytes(): retire the ad-hoc 1-second jiffies window in favor of rng_invalidation_post_check() as the single bounding authority -- EAGAIN retries, anything else is terminal (-ETIMEDOUT/-EBUSY/-EINTR from the post-check) -- matching the user-facing paths. * WC_DRBG_NEXT_STIR_LEN -> WC_RNG_NEXT_STIR_LEN ripple in wc_mix_pool_bytes(); root stats label "wc_entropyd root" -> "default bank root" (the stats are the bank's root's, daemon or no daemon). * wc_lkm_LockMutex(): latch wc_linuxkm_can_block() once at entry (the caller's context is the disposition that matters, and the function itself toggles preemption), and on CONFIG_PREEMPT_RT kernels skip the post-trylock preempt_enable() -- RT spin_trylock() does not disable preemption, so re-enabling underflowed the preempt count.
linuxkm/lkcapi_sha_glue.c, doc/dox_comments/header_files/rng_bank.h: Serialize whole-object RNG-bank transitions against invalidation walks: add WC_RNG_BANK_HAVE_INST_OP_GATE (present whenever a root_rng or the next-seed banker is), regating inst_op_gate from WC_RNG_HAVE_NEXT_SEED; bracket wc_rng_bank_invalidate_entropy()'s full walk with a spin-acquired gate claim, excluding it from instance and root free/reinstantiate cycles; add wc_rng_bank_root_rng_reinit(), a gate-bracketed retire-and-reinstantiate used by the linuxkm entropy daemon's post-invalidation last-ditch arm; fix a gate leak in wc_rng_bank_inst_reinit()'s lock-read failure path.
…DE* asserts to assure that wc_Sha256Reset() and wc_Sha512Reset() are the no-heap-operation in-place implementations in those builds. wolfcrypt/src/sha256.c: gate the in-place wc_Sha256Reset() on !WOLF_CRYPTO_CB_ONLY_SHA256.
…PREEMPT_RT NMI (rt_spin_trylock() forbidden from NMI).
linuxkm/lkcapi_sha_glue.c: in wc_linuxkm_entropy_daemon(), relax the invalidation-time root_rng reseed loop with an actual sleep, to prevent flat-out retry spinning. wolfcrypt/src/random.c: in wc_FreeRng(), wipe rng->nextStir. wolfcrypt/src/sha3.c: fix wc_Shake128_Reset() and wc_Shake256_Reset() to properly set shake->hashType.
…recognize WC_NO_COMPAT_H_REMAPS, and use it to inhibit false kernel symbol exports.
linuxkm/lkcapi_sha_glue.c: in wc_linuxkm_drbg_generate_tfm(), break out of the loop unconditionally if !WC_LINUXKM_HAVE_RNG_INVALIDATION, matching other nearby generator loops. wolfcrypt/src/random.c: * in Hash_df() and Hash512_df(), break out of the generate loop on error. * in Hash_DRBG_Generate() and Hash512_DRBG_Generate(), wipe output blocks on error (otherwise, future retry will recapitulate some of the same output bytes, since errors no longer leave mutated state). * in Hash_DRBG_StirGenerate(), map error codes from Hash*_DRBG_Generate() correctly. * in wc_RNG_GenerateBlock(), return early from failed wc_RNG_DRBG_NextSeedNow_local() for both DRBG_CONT_FIPS_E and RNG_FAILURE_E. wolfcrypt/src/sha256.c, wolfcrypt/src/sha512.c, wolfcrypt/src/sha3.c: * Don't exclude WOLFSSL_ASYNC_CRYPT from in-place (true) reset implementations -- the Free+Init_ex fallback silently dropped the async device binding, and async builds are compatible with in-place reset. * Remove frivolous asserts on in-place reset implementation when WOLFSSL_KERNEL_MODE.
…A signing is required for the v7 FIPS module" assert.
e86aaed to
73015d2
Compare
|
Tests good on bsdkm with all-crypto, all-quantum-crypto, kernel-benchmarks, crypttests, aesni, etc, with both FIPS and non-FIPS. |
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11510
Scan targets checked: linuxkm-src, linuxkm-bugs, wolfcrypt-src, wolfcrypt-bugs
Unchanged since last review (not re-run): wolfssl-src, wolfssl-bugs
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
…ct_crng_user(), map -EINTR from rng_invalidation_pre_check() to -ERESTARTSYS, for proper SA_RESTART dynamics, matching in-tree get_random_bytes_user().
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11510
Scan targets checked: linuxkm-src, linuxkm-bugs
Unchanged since last review (not re-run): wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Fenrir's latest completed scan found no issues; clearing the prior automated change request.


Fixes for RNG and linuxkm changes in #11435:
wolfcrypt/src/random.c,wolfssl/wolfcrypt/random.h,tests/unit-mcdc/test_random_whitebox.c:Make
Hash*_DRBG()failure-atomic.Eliminate the mutation seams in Hash_DRBG_Reseed/Hash512 and Hash_DRBG_Generate/Hash512: reseed derives newV and newC into scratch and commits both only after both Hash_df calls succeed; generate runs the SP 800-90A 10.1.1.4 step-2 fold ("additional") and all subsequent hashing against a shadow V, committing V/C/reseedCtr only after the last fallible step. With these refinements, a failed operation is now externally and internally indistinguishable from one never attempted, eliminating the recently introduced state_mutated condemnation machinery (and the
WC_RNG* argument it required). New contract, stated at both function pairs: no state mutation on failure.(Note that fault-injection coverage of the new contract -- no-mutation vectors and byte-identical-retry witnesses in tests/unit-mcdc/ -- lands in the following commit.)
Condemnation survives as-was at instantiation failure, stranded-lock exits, continuous-test failure, and all forced reseed paths.
The new
wc_RNG_DRBG_Reseed_Now()and wc_RNG_DRBG_NextSeedNow*() are fixed to check the DRBG reseed counter on failure, and only condemn the DRBG if the reseed was forced as inwc_RNG_GenerateBlock(), otherwise the DRBG is healthy and can continue. Reseed failures are reported to the caller regardless of condemnation.tests/unit-mcdc/test_random_fault_whitebox.c,tests/unit-mcdc/test_sp_int_whitebox.c:Add DRBG failure-atomicity fault vectors and re-measure the sp_int real-entropy sweep, for random.c's no-mutation-on-failure contract.
(Coverage companion to the preceding commit's failure-atomicity refactor of
random.c, landed separately so the contract change and its executable referee are independently reviewable; no test in the tree asserted the superseded behavior, so the intermediate state is green throughout.)In
test_random_fault_whitebox.c, addwb_hash_drbg_reseed_df_chain()/wb_hash512_drbg_reseed_df_chain()andwb_hash_drbg_generate_atomicity()/wb_hash512_drbg_generate_atomicity(). The reseed vectors fault each ofHash256_DRBG_Reseed()'s /Hash512_DRBG_Reseed()'s two derivation calls ((F,-) and (T,F)) and assertDRBG_FAILUREwith V, C, and reseedCtr byte-identical, then retry the same reseed disarmed and require clean success; vector (T,F) is the old seam's signature, where the pre-atomic code had already committed V before deriving C. The generate vectors fault the first primitive beforeHash_gen()(the 0x02 additional-input hash) and the first primitive after it (the 0x03 V-update hash, with the output block already produced) and assert no state mutation, then require the disarmed retry to produce output byte-identical to an identically-instantiated control and to land in the control's exact end state -- a failed generate consumes no stream position, and the identical retry bytes are the executable witness thatHash_gen()banks nothing across a failed attempt. Arm indices are measured viamcdc_fh_seen(), never hard-coded. Detection power was validated by temporarily reintroducing the old commit order in the SHA-256 cores (V committed between the reseed's derivations, thisV committed before the 0x03 hash): exactly the predicted assertions fired -- reseed (T,F) mutation, post-Hash_gen mutation, retry divergence -- and nothing else.Also change the TU's failure report from "done (with skips)" to "done (with failures)", the convention the rest of the suite uses and the string run-whitebox-smoke.sh greps for, so this TU's assertions (new and existing) are enforced by the smoke gate. smoke-expected.txt is unchanged: the TU was already listed and still passes.
In
test_sp_int_whitebox.c, rewrite the real-entropy sweep comment inwb_prime_trial_alloc(), which recorded condemnation-era behavior (first faulted allocation insideHash_DRBG_Generate()left theWC_RNGpermanentlyDRBG_FAILED, every later index died at the draw, :19672:0 unreachable from this sweep). Re-measured underWOLFSSL_SMALL_STACK: n = 1..2 fault the function's ownALLOC_SP_INT_ARRAYs (MP_MEM); n = 3..5 fault the draw's three allocations insideHash_DRBG_Generate()and returnRNG_FAILURE_Efor that index only, with rng.status remainingWC_DRBG_OKthroughout; n >= 6 lands inside the exponentiation and breaks the trial loop withMP_MEM, so the sweep now reaches :19672:0 as well. The pinned-RNG block remains the deterministic closer of record for that key. Comment-only change; the sweep asserts nothing.Verified with
--enable-all-crypto--enable-cryptonly--enable-static, plus--enable-smallstackand--enable-smallstack--enable-smallstackcachevariants: wolfcrypt/test/testwolfcrypt and the random/sp_int white-box TUs pass under all three, and run-whitebox-smoke.sh reports no failures and no expected-list changes.linuxkm: RNG invalidation recovery, kernel mutex behavior, error-trace fixes.
Fix error-trace machinery and add missing
WC_NO_ERR_TRACE()s: errno-domain comparisons in the PK and SHA glue and module_hooks (ENOMEM, EAGAIN, EINTR, EBADMSG, and wolfCryptINTERRUPTED_E/NOT_READY_Esites) now use the -WC_NO_ERR_TRACE(E*) idiom, with .wolfssl_known_macro_extras updated to match.Change default kernel "mutex" (spinlock) behavior: atomic-context callers now never spin -- they trylock and fail fast -- and task-context lockers re-enable preemption (and with it, migration) immediately after acquisition, rebalancing the preempt count just before release. The legacy behavior -- task-context holders promoted to non-preemptible for the duration of the hold, permitting atomic-context callers (except NMI) to spin-wait -- remains available via a new
WC_LINUXKM_SPIN_IN_ATOMICmacro, which is#error-incompatible withCONFIG_PREEMPT_RT; the mutex state field (.irq_flags vs .preempt_reenabled) follows the selected regime, andwc_lkm_UnlockMutex()moves out of line (with a pie redirect table entry).Add pre- and post-checks bracketing DRBG operations against the invalidation recovery window (keyed off last_invalidation_at, stamped at invalidation). The pre-check works as a waiting room, keeping callers from generating a CAS scan storm during the invalidation recovery interval (during which there are no usable RNGs system-wide). In the post-check, -EAGAIN passes through for another lap, a window overrun returns -ETIMEDOUT, atomic-context callers get one-shot -EBUSY (blockability is checked before signal state, because in hardirq context
currentis an unrelated task), pending signals return -EINTR, and otherwise the caller relaxes and retries. Both checks have stubs so reduced configurations build, and in-window messaging is ratelimited.Add emergency reinitialization of the entropy daemon's root_rng: when the recovery sweep finds the root invalidated, the daemon retries
wc_RNG_DRBG_Reseed_Now()with an uncredited timestamp nonce while the failure remains retryable (BUSY_E,NOT_READY_E,ENTROPY_RT_E,ENTROPY_APT_E), relaxing between attempts; for the default bank the reseed runs with preemption disabled to maximize its chances. If the reseed fails terminally, the daemon makes one last-ditch recovery viawc_FreeRng()andwc_rng_bank_root_rng_init()(orwc_InitRng()for a standalone root); a successful reinit always clears the invalidated state. If that too fails, the daemon exits, leaving bank instances to self-reseed. Outcomes are reported at severities differentiated by default vs non-default bank (pr_notice/pr_info success, pr_emerg/pr_err failure). The daemon also now supports a standalone malloc'd root_rng where no bank features are built, instantiated at startup and freed on exit.Also:
wc_linuxkm_drbg_generate_tfm(): retry loop around the invalidation window replaces the single-shot call; output buffer is zeroized on terminal error.wc__get_random_bytes(): honors the recovery window, and zeroizes its staging buffer both before retry re-entry and on the error exit. An invalidation may be peculiar to this VM and leave the bytes live in another VM, so they are wiped when abandoned, as possible CSP elsewhere.wc_linuxkm_entropy_daemon(): the registered-leaf recovery pass is now additionally gated onWC_LINUXKM_HAVE_RNG_REGISTRY, matching its actual dependency; seed health test failures (ENTROPY_RT_E/ENTROPY_APT_E) on bank instances are now reported unconditionally at pr_warn_ratelimited severity ("entropy source suspect"), rather than pr_err only underWC_VERBOSE_RNG; theWC_RNG_DEBUG_STATSdump at daemon exit is removed (was redundant).wolfcrypt/test/test.c: SVR access-counter checks now additionally gated on newWC_SVR_HAVE_FLAGS, matching the configurations that actually maintain the counters.tested atop #11509 with: