You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
wolfcrypt/src/rng_bank.c and wolfssl/wolfcrypt/rng_bank.h: backward-compat and !WC_RNG_HAVE_NEXT_SEED tweaks.
Add backward-compat implementations of wc_rng_bank_inst_lock_{get,put}_unconditional();
Fix gating around WC_RNG_INIT_FLAG_UPGRADE_FROM_NEXT_SEED
(defined(WC_RNG_INIT_FLAG_UPGRADE_FROM_NEXT_SEED) -> defined(WC_RNG_HAVE_NEXT_SEED)) (_HAVE_NEXT_SEED is the requisite
feature-sensing flag; the _INIT_FLAG_ is always defined).
linuxkm/lkcapi_sha_glue.c: complete support for !WC_RNG_HAVE_NEXT_SEED in wc_linuxkm_entropy_daemon().
In wc_linuxkm_entropy_daemon(), add !WC_RNG_HAVE_NEXT_SEED code paths for the
bank leaf recovery pass, using wc_RNG_DRBG_Reseed_Now();
Change the WC_RNG_HAVE_NEXT_SEED path to use wc_RNG_DRBG_NextSeedGenerate()
directly to pick up lineage from the RNG-level mechanics;
Similar refactor in the "reseed pass for long-lived leaves", adding
!WC_RNG_HAVE_NEXT_SEED implementation, plus use wc_RNG_entropy_needs_recovery()
to determine eligibility, and change the section build gate from
(WC_LINUXKM_HAVE_RNG_REGISTRY && WC_RNG_HAVE_NEXT_SEED && WC_RNG_HAVE_RBGC) to
(WC_LINUXKM_HAVE_RNG_REGISTRY && WC_RNG_HAVE_ENTROPY_EPOCH).
Similar refactor in the "bank leaf bonus reseed pass";
Add proper gating on WC_RNG_BANK_FLAG_AUTO_UPGRADE_FROM_NEXT_SEED in the wc_rng_bank_recover_inst() call.
Scan targets checked:linuxkm-src, linuxkm-bugs, wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs Coverage: 3 of 3 in-scope changed file(s) opened by the reviewer
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.
…ompat and !WC_RNG_HAVE_NEXT_SEED tweaks.
* Add backward-compat implementations of
wc_rng_bank_inst_lock_{get,put}_unconditional();
* Fix gating around WC_RNG_INIT_FLAG_UPGRADE_FROM_NEXT_SEED
(defined(WC_RNG_INIT_FLAG_UPGRADE_FROM_NEXT_SEED) ->
defined(WC_RNG_HAVE_NEXT_SEED)) (_HAVE_NEXT_SEED is the requisite
feature-sensing flag; the _INIT_FLAG_ is always defined).
…D in wc_linuxkm_entropy_daemon().
* In wc_linuxkm_entropy_daemon(), add !WC_RNG_HAVE_NEXT_SEED code paths for the
bank leaf recovery pass, using wc_RNG_DRBG_Reseed_Now();
* Change the WC_RNG_HAVE_NEXT_SEED path to use wc_RNG_DRBG_NextSeedGenerate()
directly to pick up lineage from the RNG-level mechanics;
* Similar refactor in the "reseed pass for long-lived leaves", adding
!WC_RNG_HAVE_NEXT_SEED implementation, plus use wc_RNG_entropy_needs_recovery()
to determine eligibility, and change the section build gate from
(WC_LINUXKM_HAVE_RNG_REGISTRY && WC_RNG_HAVE_NEXT_SEED && WC_RNG_HAVE_RBGC) to
(WC_LINUXKM_HAVE_RNG_REGISTRY && WC_RNG_HAVE_ENTROPY_EPOCH).
* Similar refactor in the "bank leaf bonus reseed pass";
* Add proper gating on WC_RNG_BANK_FLAG_AUTO_UPGRADE_FROM_NEXT_SEED in the
wc_rng_bank_recover_inst() call.
…nts/header_files/rng_bank.h: add wc_rng_bank_root_rng_reseed_now() and wc_rng_bank_root_rng_reseed_if_due().
Serialized forced reseed of the bank root, by whatever method the configuration
uses to serialize root access: with WC_RNG_HAVE_LOCK, the root's lease is taken
by unconditional try-lock and held across the reseed; without it, the inst-op
gate is claimed as WC_RNG_BANK_INST_OP_DAEMON per the holder contract (try-once,
fail-fast, atomic-legal), excluding member reinstantiations chaining from the
root, entropy-invalidation walks, and root reinstantiation. Either refusal
returns BUSY_E with nothing touched.
The threshold form reads the reseed counter under the same hold as the reseed --
no TOCTOU, and on legacy FIPS boundaries no unexcluded dereference of internal
DRBG state -- and returns ALREADY_E (steady state, not an error) below
threshold; threshold 0 reseeds unconditionally. Without the lock facility the
saturated counter is the invalidation walk's only root-side record, so a
threshold of WC_RESEED_INTERVAL implements deterministic post-invalidation root
recovery.
…wc_linuxkm_vmgenid_poll() in !WC_RNG_HAVE_NEXT_SEED and legacy-FIPS configurations.
legacy-FIPS (!WC_RNG_HAVE_ENTROPY_EPOCH) bank-leaf sensing and recovery:
* recovery pass: sense invalidation directly on the wc_rng_bank_inst.lock
stand-in latch via wc_rng_bank_inst_lock_read(), replacing the
reseedCtr-overload check. The latch is the authoritative record --
invalidation during a consumer's hold leaves the counter unsaturated --
and the one-word read eliminates the unexcluded dereference of internal
DRBG state in wc_RNG_DRBG_GetReseedCtr() on legacy boundaries.
* recovery and bonus passes: perform the reseed with
wc_rng_bank_inst_reseed_now() under
wc_rng_bank_inst_lock_get_unconditional(), so a successful reseed
clears the inst latch (recovery_exit) rather than stranding an instance
that ordinary checkouts refuse, and the try-lock excludes in-flight
holders and reinits. In the bonus pass, take the inst lock before
wc_RNG_DRBG_GetReseedCtr(), for the same legacy-dereference reason, and
release it on every path.
* add #error tripwires for WC_RNG_HAVE_LOCK or WC_RNG_HAVE_NEXT_SEED
without WC_RNG_HAVE_ENTROPY_EPOCH (unreachable kernel module
configurations) and drop the resulting dead arms, unifying the recovery
action across epoch and legacy configurations. Drop double-counted
verbose accounting in the recovery action (the shared chain below
covers all return codes).
registry-leaf pass (!WC_RNG_HAVE_NEXT_SEED):
* inhibit inline reseeds of leaves that aren't lock-enforcing: a leaf
created without WC_RNG_INIT_FLAG_LOCK_REQUIRED has owners that never
consult WC_RNG.lock, so wc_RNG_lock_get_unconditional() excludes nobody
and the daemon's inline reseed would update DRBG state concurrently
with the owner's generates. Sense WC_RNG_LOCK_REQUIRED via
wc_RNG_lock_read() and skip the leaf otherwise.
wc_RNG_entropy_needs_recovery() has already propagated the invalidation
event by the time the gate is evaluated, so a skipped leaf recovers in
its owner's next generate; only the daemon-side inline reseed is
inhibited. Bank leaves are unaffected (always _LOCK_REQUIRED or the
wc_rng_bank_inst.lock stand-in equivalent, and accessed only through
the bank checkout API), and WC_RNG_HAVE_NEXT_SEED configurations
synchronize through the WC_RNG.nextSeedLen aperture instead.
root_rng recovery and serialization in !WC_RNG_HAVE_LOCK configurations:
* the loop-head root recovery block gains an #else arm: a bank root
recovers via wc_rng_bank_root_rng_reseed_if_due() at threshold
WC_RESEED_INTERVAL -- the invalidation walk's saturation is the only
root-side record in this configuration, and the threshold form reads
and acts under a single hold of the inst-op gate -- bounding recovery
latency at one daemon nap, as in the lock-facility arm. A memberless
root checks wc_RNG_entropy_needs_recovery() when
WC_RNG_HAVE_ENTROPY_EPOCH (the global epoch detects without
notification); without the epoch facility nothing can reach a
daemon-private root, and the vmgenid fold plus the periodic countdown
reseed are its complete freshness regime (now documented).
* the periodic root reseed routes through
wc_rng_bank_root_rng_reseed_now() on lock-less bank roots, excluding
member reinstantiations that chain from the root, and now passes the
cycle-counter nonce on all boundaries, replacing the pre-v7 NULL arm:
the nonce lands natively in the credited reseed on current code and the
5.2.4 backport, and as a trailing uncredited stir in the
legacy-boundary fallback.
* wc_linuxkm_vmgenid_poll(): the generation-id fold routes through
wc_rng_bank_root_rng_reseed_now() whenever the bank has a root,
retiring the hand-rolled lease bracket; the signature takes the bank
when WC_RNG_BANK_HAVE_ROOT_RNG. The memberless arm is documented as
daemon-private with no reachable invalidation events.
* vmgenid poll teardown is no longer gated on root_rng != NULL: the poll
runs (and maps the ACPI generation-id page) regardless of the root's
fate, so the previous gating leaked the mapping on the
continuing-without-root path.
* note added at the loop-head needs_recovery() check: any nonzero verdict
deliberately routes to recovery -- a dire code is exactly when the
reinit arm is wanted.
pass accounting and tidying:
* the congestion-triggered pre-bank pass sets congested_progress only on
success: a persistently failing wc_rng_bank_next_seed_generate_rbgc()
no longer suppresses the banking, bonus, and periodic reseed passes
indefinitely.
* harmonize the global-fallback bonus threshold to
>= WC_LINUXKM_BONUS_RESEED_INTERVAL, matching the leaf passes.
* note that wc_RNG_global_fallback_get() returns the pointer to the
static WC_RNG, not a hold; fix a doubled space in its error message.
* normalize nested preprocessor-directive indentation in the root
recovery block; label the bonus passes by boundary; misc comment fixes.
Scan targets checked:linuxkm-src, linuxkm-bugs, wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs Not reviewed (no scan target covers them):doc/dox_comments/header_files/random.h, doc/dox_comments/header_files/rng_bank.h Coverage: 3 of 3 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
…einstantiation, recovery-pass gating, and epoch-less configuration support.
* !WC_RNG_HAVE_LOCK root recovery: An out-of-service root cannot recover by
reseed (the reseed paths refuse a failed instance), so the recovery arm now
leads with a lockless status peek and reinstantiates via
wc_rng_bank_root_rng_reinit() (gate-serialized internally; spin-entered, task
context). Likewise for a memberless root, by bare free+reinstantiate
(daemon-private, no serialization needed or possible), covering the epoch-less
memberless case nothing else could notice. Unlike the lock-facility arm, a
hard reinit failure does not exit the daemon: without the lock facility the
members cannot self-reseed inline, so the daemon stays and retries.
* Gate the bank-leaf invalidation recovery pass on HAVE_HASHDRBG rather
than WC_RNG_HAVE_RBGC: invalidation never saturates the reseed counter
on lock-facility builds, so without this pass a healthy-but-latched
leaf in a !WC_RNG_HAVE_NEXT_SEED && !WC_RNG_HAVE_RBGC build is
invisible to the bonus pass and the status patrol alike, and ordinary
checkouts starve. The reseed is implicitly RBGC when the instance has
a chain parent, else primary-sourced; RBGC was never a precondition.
* Remove the just-added #error tripwires for
WC_RNG_HAVE_LOCK/WC_RNG_HAVE_NEXT_SEED without WC_RNG_HAVE_ENTROPY_EPOCH:
WC_RNG_NO_ENTROPY_EPOCH (random.h) disables the epoch independently of the
lock and next-seed facilities, so those configurations are reachable and
buildable. No fallback arms need restoring: the wc_rng_bank_inst lock
accessors map to WC_RNG.lock when the lock facility is present and to the
wc_rng_bank_inst.lock stand-in otherwise, and the reseed epilogue's latch
clear is epoch-independent, so the unified epoch-less arm already serves all
of them.
Add tests for root reseed thresholds and busy-state recovery
wolfcrypt/src/rng_bank.c:1686
Neither new root-reseed API is exercised by the automated tests, although wolfcrypt/test/test.c already tests root reinitialization and invalidation. Please add focused tests for below/equal-threshold counters, threshold 0, and a busy root lease or inst-op gate leaving the counter unchanged. These are the new serialization and recovery guarantees that the existing integration smoke tests do not directly assert.
…covery-coverage gaps in epoch-less and lock-less configurations.
* bank recovery pass, epoch arm: on !WC_RNG_HAVE_LOCK, also read the
wc_rng_bank_inst.lock stand-in latch, which
wc_RNG_entropy_needs_recovery() cannot see -- a member invalidated
while checked out is latched without counter saturation, and a
bank-scoped invalidation moves no global epoch, so the member read as
healthy while checkouts refused it. RNG-level verdicts are preserved.
* registry-leaf sweep: gate on
WC_LINUXKM_HAVE_RNG_REGISTRY && (WC_RNG_HAVE_LOCK ||
WC_RNG_HAVE_NEXT_SEED) rather than the entropy epoch, which is neither
necessary (needs_recovery reads the latch epoch-independently) nor
sufficient (the !_NEXT_SEED arm is lock-facility API). The gate admits
exactly the configurations where the sweep has a safe action, and makes
the lock_read arm compile-safe by construction.
Add regression tests for root reseeding and failure cleanup
wolfcrypt/src/rng_bank.c:1686
The new root-reseed services have no regression tests, although wolfcrypt/test/test.c already tests root reinitialization and RNG-bank contention. Add cases for below-threshold ALREADY_E without mutation, reseeding at the threshold, threshold-zero unconditional reseeding, and BUSY_E without mutation when the root lease or legacy inst-op gate is held. Verify that reseed failures release the serialization primitive so later recovery can proceed.
…read races on the unowned-caller path, and a lost registry recovery signal.
* random.c: wc_RNG_DRBG_NextSeedGenerate() admits unowned callers by
design (a scheduling daemon synchronizes through the next-seed
aperture, not the instance lease), so a concurrent teardown of rng can
CAS-null rng->RBGC_parent mid-call: a reread between wc_RNG_lock_get()
and wc_RNG_lock_put() could release the wrong object (or none),
leaving the parent permanently locked, and the rereads in the FIPS
eligibility criteria (the fallback arm, and the source check in
_NextSeedGenerate_local(), reachable from the same entry) could
dereference null. Capture the pointer exactly once per scope; the
held lease pins the parent shell against its owner's wc_FreeRng().
The owned-rng reseed paths (PollAndReSeed(),
wc_RNG_DRBG_ReseedRBGC_local()) are unchanged: ownership of rng is
required there by mechanism or contract, which makes RBGC_parent
stable, and the comments now record that distinction.
* lkcapi_sha_glue.c: the registry-leaf sweep no longer clears
wc_linuxkm_rng_registry_needs_recovery after a pass in which a leaf
was refused on a transient (lock contention, source not ready, or a
standing latch): a lock-required leaf's own consumers are refused by
the same latch, so the sweep is its only recovery agent, and clearing
the counter consumed the only signal that brings the sweep back.
Leaves the sweep can never serve (lock-naive) do not hold the counter
hot.
Add regression tests for root-reseed synchronization and recovery
wolfcrypt/src/rng_bank.c:1659
The new root-reseed APIs have no regression coverage, although wolfcrypt/test/test.c already tests root initialization/reinitialization with and without WC_RNG_HAVE_LOCK. Please extend those tests to cover below/at-threshold behavior, threshold zero, BUSY_E without state changes when the lease or gate is held, and recovery after counter saturation. These are new synchronization and recovery guarantees that module-load smoke tests do not directly verify.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
wolfSSL-Fenrir-bot
left a comment
wolfcrypt/src/rng_bank.candwolfssl/wolfcrypt/rng_bank.h: backward-compat and !WC_RNG_HAVE_NEXT_SEEDtweaks.Add backward-compat implementations of
wc_rng_bank_inst_lock_{get,put}_unconditional();Fix gating around
WC_RNG_INIT_FLAG_UPGRADE_FROM_NEXT_SEED(
defined(WC_RNG_INIT_FLAG_UPGRADE_FROM_NEXT_SEED)->defined(WC_RNG_HAVE_NEXT_SEED))(_HAVE_NEXT_SEEDis the requisitefeature-sensing flag; the
_INIT_FLAG_ is always defined).linuxkm/lkcapi_sha_glue.c: complete support for !WC_RNG_HAVE_NEXT_SEEDinwc_linuxkm_entropy_daemon().In
wc_linuxkm_entropy_daemon(), add !WC_RNG_HAVE_NEXT_SEEDcode paths for thebank leaf recovery pass, using
wc_RNG_DRBG_Reseed_Now();Change the
WC_RNG_HAVE_NEXT_SEEDpath to usewc_RNG_DRBG_NextSeedGenerate()directly to pick up lineage from the RNG-level mechanics;
Similar refactor in the "reseed pass for long-lived leaves", adding
!
WC_RNG_HAVE_NEXT_SEEDimplementation, plus usewc_RNG_entropy_needs_recovery()to determine eligibility, and change the section build gate from
(
WC_LINUXKM_HAVE_RNG_REGISTRY&&WC_RNG_HAVE_NEXT_SEED&&WC_RNG_HAVE_RBGC) to(
WC_LINUXKM_HAVE_RNG_REGISTRY&&WC_RNG_HAVE_ENTROPY_EPOCH).Similar refactor in the "bank leaf bonus reseed pass";
Add proper gating on
WC_RNG_BANK_FLAG_AUTO_UPGRADE_FROM_NEXT_SEEDin thewc_rng_bank_recover_inst()call.tested with
with and without
-DWC_RNG_NO_NEXT_SEED -DWC_RNG_NO_POOL'