[LinuxJedi]

Fixes WP11_Lock releasing a lock from a thread that did not take it, fixes the remaining races ThreadSanitizer then reports, and turns TSAN back on in CI. This supersedes #251 and #253; #252 has been merged separately.

The branch has master merged in (56be05a). Since it was opened, master gained a deferred free for destroyed objects (WP11_Slot_DiscardObject) and its own fix for the empty-PIN cache (F-8673), which cover the WP11_Object_Find use-after-free and the WP11_Slot_Has_Empty_Pin race this PR originally fixed. The merge takes master's versions of both, so the diff against master no longer touches them.

Stop releasing WP11_Lock from a thread that did not take it

WP11_Lock is the reader/writer lock used for the global, slot and token state. Its read path locked the write mutex for the first reader and unlocked it for the last reader, which is often a different thread. Releasing a pthread mutex, or leaving a critical section, from a thread that does not own it is undefined behaviour:

• Under Wine, pkcs11mtt hangs: with the old lock it hung until a timeout in 4 of 5 runs, with this change it passed 8 of 8.
• ThreadSanitizer cannot track the lock, so a TSAN make check reports hundreds of false races and lock-order inversions, even in single-threaded tests.

The lock is now one mutex plus a condition variable, with a reader count and a writer flag. The mutex only guards the counters and is never held while the lock is held. The semantics are unchanged: a waiting writer does not stop new readers, so a thread can take the read lock again while holding it.

• pthreads (WOLFSSL_PTHREADS): pthread_mutex_t and pthread_cond_t. The wait runs with cancellation disabled, so acquiring the lock is still not a cancellation point.
• Windows (USE_WINDOWS_API, Vista and later): CRITICAL_SECTION and CONDITION_VARIABLE. A multi-threaded Windows build for Windows CE or with _WIN32_WINNT below 0x0600 now fails with an #error instead of falling back to the old lock.
• Other threading APIs keep the two-mutex lock as a fallback, with a comment that it is only correct where a mutex may be released by another thread (for example a binary semaphore). WP11_NO_LOCKING builds are unchanged.

Fix remaining TSAN races and enable TSAN in sanitizer CI

• session->curr: WP11_Slot_ClearActiveObject (from C_DestroyObject) reads and clears every session's active object under the slot lock, while each session's *Init wrote its own through WP11_Session_SetObject with no lock. WP11_Session_GetObject and WP11_Session_SetObject now take the slot lock as readers; GetObject decides whether to abort an operation whose key was destroyed under the lock and calls WP11_Session_AbortOp after releasing it. Their callers are in crypto.c and hold no locks.
• Test harness: tests/unit.h had its own copy of the cross-thread-unlock read/write lock; it is now a start gate plus a mutex-guarded stop flag. concurrent_destroy_object_test uses __atomic acquire/release for its spin barrier instead of volatile.
• sanitizer-tests.yml: tsan is back in the matrix; the TSAN_OPTIONS export pointing at a tsan_suppressions.txt that has never existed is removed (TSAN aborts every test at startup when its suppressions file is missing); and vm.mmap_rnd_bits is lowered for TSAN only, since its fixed shadow mappings can collide with the higher ASLR entropy of current runner kernels.

Empty-PIN cache test

tests/empty_pin_cache_test drives the cache-discard branch of WP11_Slot_Has_Empty_Pin deterministically: a DEBUG_WOLFPKCS11-only hook changes the PIN from empty to non-empty between the uncached check and caching its result, then the test checks a CKA_PRIVATE create in the public session is refused and is allowed after logging in. With the seed comparison removed it fails. It needs WP11_MIN_PIN_LEN=0, so it runs in the empty PIN store workflow and skips elsewhere.

Testing:

• Pre-commit hook make test passes on every commit (debug and default configs).
• After merging master: macOS TSAN make check (debug PQC) clean, 77 pass / 5 skip, including the new threaded login and logout-retire tests; ASAN make check with WP11_MIN_PIN_LEN=0, 79 pass / 3 skip.
• Before the merge, the TSAN job was replicated in a Linux container (Debian trixie aarch64, clang 19, wolfSSL built with TSAN from the workflow's configure line, TSAN wolfTPM and ibmswtpm2 for the TPM configs): all five matrix configs clean; master in the same job failed 53 of 62 tests.
• Windows: MinGW-w64 (ucrt64) static cross-build run under Wine: 58 pass, 4 skip, 0 fail. The #error was checked with -D_WIN32_WINNT=0x0501. MSVC was not built locally; win-test.yml covers it.
• Interleaved A/B timing of the old and new lock (concurrent_destroy_object_test, pkcs11mtt): no measurable difference.

[Copilot]

🔵 Needs a closer look

Cross-platform locking and authorization-sensitive PIN caching need human concurrency review, including the missing cache-invalidation regression test.

1 open finding

• Add regression test for stale empty-PIN cache after PIN change · New

What changed in this PR

Addresses synchronization races in wolfPKCS11 and its test harness, and restores ThreadSanitizer (TSAN) CI coverage. Includes the prerequisite lock and object-lookup fixes from #253 and #251.

Changes:

• Updates library locking and protects active-object and PIN-cache access.
• Synchronizes test startup, stopping, and concurrent-destroy barriers.
• Enables TSAN with runner memory-layout adjustments and removes the missing suppression-file reference.

File	Description
tests/​unit.h	Replaces harness locking with a start gate and synchronized stop flag.
tests/​concurrent_destroy_object_test.c	Uses atomic operations for thread coordination.
src/​internal.c	Fixes lock ownership, object lookup, and shared-state synchronization.
.github/​workflows/​sanitizer-tests.yml	Restores TSAN and adjusts its runner configuration.

🧠 Review effort: Balanced

---

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

[Frauschi]

This needs a rebase: since it branched, master picked up #227/#243/#252, and there are conflicts in src/internal.c, wolfpkcs11/internal.h and empty-pin-store-test.yml. The lock rewrite now has to fit around WP11_NO_LOCKING (aebbd51).

Also worth checking during the rebase: master now defers freeing destroyed objects until no object call can still be using them (WP11_Slot_DiscardObject, from ba08cd4). That looks like it already covers the WP11_Object_Find use-after-free, so b6cf59c may no longer be needed, or could be reduced to the CKA_PRIVATE capture.

[LinuxJedi]

@Frauschi thanks. I merged master into the branch (56be05a) rather than rebasing, to avoid force-pushing an open PR; happy to rebase instead if you'd prefer a linear history.

• WP11_Lock: the condition-variable lock now sits behind WP11_NO_LOCKING, for both the type and the functions; the fallback is guarded by !WP11_NO_LOCKING.
• WP11_Object_Find: agreed. WP11_Slot_DiscardObject keeps a destroyed object allocated until the losing destroyer's object call has ended, and master already captures opFlag under the token lock, so I took master's version. b6cf59c no longer changes anything; the branch's diff against master doesn't touch WP11_Object_Find.
• WP11_Slot_Has_Empty_Pin: master's F-8673 change does what this branch did (only cache the verdict if the PIN didn't change during the check, keyed on the seed), so I took master's version. empty_pin_cache_test and its hook stay: they drive that discard branch deterministically, and the test fails against master's code if the seed comparison is removed.
• WP11_Session_GetObject: keeps master's abort of an operation whose key was destroyed, but reads session->curr and the abort condition under the slot lock, and calls WP11_Session_AbortOp after releasing it.

After the merge: pre-commit hook make test passes (debug 77 pass / 5 skip, default 74 / 8); macOS TSAN make check (debug PQC) is clean, 77 pass / 5 skip, including the new threaded login and logout-retire tests; ASAN make check with WP11_MIN_PIN_LEN=0 is 79 pass / 3 skip.