Fix entropy_mutex ownership tracking & re-init race by stenslae · Pull Request #11645 · wolfSSL/wolfssl

stenslae · 2026-10-05T18:35:52Z

Description

Fixed fenrir findings for F-7091, F-7419, and F-8169.

Added boolean tracking on the mutex lock, and made it so mutex locking and stop thread are only called when the current thread successfully acquired the lock. Also changed wc_InitMutex() in Entropy_Init() with wc_local_InitMutexOnce() to prevent re-initalization of an active mutex, and added resetting the atomic entropy flag in Entropy_Final().

Testing

Added whitebox vectors for re-initialization cycles, mutex lock refusals, and held-mutex cleanup after internal failures

Checklist

added tests
updated/added doxygen
updated appropriate READMEs
Updated manual and documentation

github-actions · 2026-10-05T18:39:06Z

Copilot

Copilot review overview

🔵 Needs a closer look

Entropy-source synchronization changes need platform-aware human review and stronger concurrency regression coverage.

Review effort: Balanced
Findings: 2

Open (2)

Add concurrent Entropy_Init regression using real atomic once logic · New
Verify refused entropy locks do not stop another caller's counter · New

What changed in this PR

Hardens wolfCrypt’s memory-based entropy source against mutex ownership and re-initialization errors.

Changes:

Guards mutex release and counter-thread shutdown with ownership tracking.
Uses one-time mutex initialization and resets its flag during finalization.
Adds whitebox lifecycle and failure-path tests.

File	Description
wolfcrypt/src/wolfentropy.c	Updates mutex ownership, initialization, and cleanup handling.
tests/unit-mcdc/test_wolfentropy_whitebox.c	Adds re-initialization, lock-refusal, and held-mutex cleanup checks.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11645

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-bugs
Coverage: 2 of 2 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.

Review tier: Lite

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11645

Scan targets checked: wolfssl-bugs
Unchanged since last review (not re-run): wolfcrypt-src, wolfcrypt-bugs
Coverage: 1 of 1 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.

Review tier: Lite

Copilot

Copilot review overview

🟡 Changes recommended

The new on-demand test baseline can falsely report a regression when the threaded entropy counter is stopped.

Review effort: Balanced
Findings: 1

Open (1)

Restart entropy counter before on-demand test to avoid false failures · New

Resolved since last review (2)

Verify refused entropy locks do not stop another caller's counter
Add concurrent Entropy_Init regression using real atomic once logic

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11645

Scan targets checked: wolfssl-bugs
Unchanged since last review (not re-run): wolfcrypt-src, wolfcrypt-bugs
Coverage: 1 of 1 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.

Review tier: Lite

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review effort: Lite
Findings: 5

Open (5)

wb_regress is used as the “real failure” indicator, but the name is ambiguous (it reads like… · New
wb_entropy_get_mutex() reports a skip via WB_NOTE(...) but doesn’t set any flag that affects… · New
In production code, Entropy_StartThread() / Entropy_StopThread() are invoked while holding… · New
Same issue as in wb_entropy_get_mutex(): this early-return “skip” path is not reflected in the… · New
wb_regress is used as the “real failure” indicator, but the name is ambiguous (it reads like… · New

Resolved since last review (1)

Restart entropy counter before on-demand test to avoid false failures

Copilot

🔵 Needs a closer look

Entropy-source synchronization needs platform-specific maintainer validation, and assertion-failure reporting remains unresolved.

1 open finding

Report ownership assertion failures with the recognized failure marker · New

5 resolved since last review

wb_regress is used as the “real failure” indicator, but the name is ambiguous (it reads like…
Same issue as in wb_entropy_get_mutex(): this early-return “skip” path is not reflected in the…
In production code, Entropy_StartThread() / Entropy_StopThread() are invoked while holding…
wb_entropy_get_mutex() reports a skip via WB_NOTE(...) but doesn’t set any flag that affects…
wb_regress is used as the “real failure” indicator, but the name is ambiguous (it reads like…

🧠 Review effort: Balanced

+    if (ret != WC_NO_ERR_TRACE(BAD_MUTEX_E) || wb_unlocks != 1) {
+        WB_NOTE("Entropy_Init left entropy_mutex held");
+        wb_fail = 1;


wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11645

Scan targets checked: wolfssl-bugs
Unchanged since last review (not re-run): wolfcrypt-src, wolfcrypt-bugs
Coverage: 1 of 1 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.

Review tier: Lite

… the counter thread or unlocking

Copilot

🟡 Changes recommended

Initialization readiness can be published before startup completes, allowing a concurrent caller to return success prematurely.

2 open findings

🧠 Review effort: Balanced

+            /* Short circuit return -- a competing thread initialized the
+             * state while we were waiting. Note: threadsafe when
+             * WOLFSSL_MUTEX_INITIALIZER or WOLFSSL_ATOMIC_OPS is available.


wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11645

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-bugs
Coverage: 2 of 2 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.

Review tier: Lite

Fenrir's latest completed scan found no issues; clearing the prior automated change request.

stenslae requested a review from wolfSSL-Fenrir-bot October 5, 2026 18:35

stenslae self-assigned this Oct 5, 2026

Copilot AI balanced review requested due to automatic review settings October 5, 2026 18:35

Copilot started reviewing on behalf of stenslae October 5, 2026 18:36 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread tests/unit-mcdc/test_wolfentropy_whitebox.c Outdated

Comment thread tests/unit-mcdc/test_wolfentropy_whitebox.c Outdated

wolfSSL-Fenrir-bot reviewed Oct 5, 2026

View reviewed changes

stenslae force-pushed the audit-entropy-mutex branch from 3479616 to 9c11a59 Compare October 5, 2026 19:39

stenslae requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot October 5, 2026 19:39

wolfSSL-Fenrir-bot reviewed Oct 5, 2026

View reviewed changes

Copilot started reviewing on behalf of stenslae October 5, 2026 19:50 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

Comment thread tests/unit-mcdc/test_wolfentropy_whitebox.c Outdated

stenslae force-pushed the audit-entropy-mutex branch from 9c11a59 to 1b65c7a Compare October 5, 2026 20:02

stenslae requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot October 5, 2026 20:02

wolfSSL-Fenrir-bot previously requested changes Oct 5, 2026

View reviewed changes

Comment thread tests/unit-mcdc/test_wolfentropy_whitebox.c Outdated

Copilot AI reviewed Oct 5, 2026

View reviewed changes

stenslae force-pushed the audit-entropy-mutex branch from 1b65c7a to eb4b2dd Compare October 7, 2026 21:32

stenslae requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot October 7, 2026 21:35

Copilot started reviewing on behalf of stenslae October 7, 2026 21:36 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

Comment thread tests/unit-mcdc/test_wolfentropy_whitebox.c Outdated

Comment on lines +496 to +498

if (ret != WC_NO_ERR_TRACE(BAD_MUTEX_E) || wb_unlocks != 1) {

WB_NOTE("Entropy_Init left entropy_mutex held");

wb_fail = 1;

wolfSSL-Fenrir-bot previously requested changes Oct 7, 2026

View reviewed changes

Comment thread tests/unit-mcdc/test_wolfentropy_whitebox.c Outdated

Emma Stensland added 2 commits October 9, 2026 13:53

F-7091, F-7419, F-8169: track entropy_mutex ownership before stopping…

8d8ef55

… the counter thread or unlocking

Fix Entropy_Init() re-init race

52173d9

stenslae force-pushed the audit-entropy-mutex branch from eb4b2dd to 52173d9 Compare October 9, 2026 19:59

stenslae requested review from wolfSSL-Fenrir-bot and a balanced review from Copilot October 9, 2026 20:00

Copilot started reviewing on behalf of stenslae October 9, 2026 20:00 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread wolfcrypt/src/wolfentropy.c

Comment on lines +1020 to +1022

/* Short circuit return -- a competing thread initialized the

* state while we were waiting. Note: threadsafe when

* WOLFSSL_MUTEX_INITIALIZER or WOLFSSL_ATOMIC_OPS is available.

wolfSSL-Fenrir-bot reviewed Oct 9, 2026

View reviewed changes

stenslae changed the title ~~Fix entropy_mutex ownership tracking, re-init race, and whitebox coverage~~ Fix entropy_mutex ownership tracking & re-init race Oct 9, 2026

Conversation

stenslae commented Oct 5, 2026

Description

Testing

Checklist

Uh oh!

github-actions Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MemBrowse Memory Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11645

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11645

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11645

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11645

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11645

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Oct 5, 2026 •

edited

Loading