ltc/node: fix kr1z1s use-after-free / SIGSEGV race (hold tracker lock in phase2 + guard notify contains) [#759 RANK-1]#791
Conversation
…ify contains (#759) Back-port the BTC/BCH lock discipline into the LTC node to close the use-after-free / SIGSEGV race (the "kr1z1s" report) that the current LTC master carries. The hold-lock discipline was originally pioneered on LTC (f445db8); LTC master since regressed to the racy early-release/bare-contains pattern while BTC and BCH kept the fix. This restores parity — the two LTC functions are now byte-identical to their BTC/BCH counterparts. processing_shares_phase2 (impl/ltc/node.cpp): - Previously acquired m_tracker_mutex under a try_to_lock in an inner scope, then RELEASED it and mutated m_tracker.chain lock-free. The compute-thread clean_tracker() exclusive prune (drop_tails) could free chain nodes mid-mutation -> UAF SIGSEGV. - Now holds the lock across the entire mutation body (function-scope unique_lock), releasing only immediately before the async run_think() post so the compute thread can take the exclusive lock. - Adds the MAX_PENDING_ADDS (256) backpressure cap on the deferred queue, matching BTC/BCH, so a wedged think() cannot grow m_pending_adds unbounded. notify_local_share (impl/ltc/node.cpp): - Previously did a bare m_tracker.chain.contains() outside any lock, racing the same prune. Now the contains() read AND attempt_verify() both run under the try_to_lock; on lock-not-owned the inline verify is skipped (the share is already in-chain; the next run_think() cycle scores it). Lock-scope + guard only. No wire / share / consensus / protocol-version / PPLNS change: the happy path is identical, and the not-owned path queues/defers work that think() drains next cycle anyway. Behaviour-preserving. Refs #759 (drift-map RANK-1).
Phase2HoldsLockAgainstComputePrune models the exact kr1z1s hazard: a compute thread takes the shared_mutex exclusively and frees heap-allocated chain nodes (drop_tails analog) while two IO threads mutate/read the same nodes under try_to_lock. The IO body holds the lock across every node access and guards contains() under the lock (the btc/bch discipline back-ported to ltc). A discipline break is a genuine use-after-free (fails under ASan/TSan). Same simulation style as the existing threading tests (no full NodeImpl).
Build + test results (local Debug, GTest)Build: Tests — all green (123 total):
Byte-parity (the fix == canonical): normalized full-body diff of LTC Consensus-neutral: diff-grep over the changed lines finds no consensus tokens (protocol/share-version, pplns, payout, target, work, coinbase, merkle, difficulty). Every changed line is a lock primitive, a comment, the backpressure branch, or re-indentation of the existing defer-queue log lines. Prod status (LTC-DOGE test node, 158.220.92.171): NOT actively crashing from this race — current c2pool process up 3d15h with 0 restarts (bare process, not systemd), zero SIGSEGV/segfault in journal or dmesg (all-time), and the only core on record is a single SIGABRT (2026-07-15, pre-dating the current 07-17 process start — consistent with the RSS-limit |
1 participant
Summary
Closes the RANK-1 latent bug from the #759 node consensus-core drift map: the "kr1z1s" use-after-free / SIGSEGV race in the LTC node. Behaviour-preserving lock-scope + guard fix — zero wire / share / consensus / protocol-version / PPLNS change.
The hold-lock discipline was originally pioneered on LTC (
f445db8e) and applied to BTC/BCH; LTC master since regressed to the racy early-release / bare-contains pattern while BTC and BCH kept the fix. This restores parity — the two LTC functions are now byte-identical to their BTC/BCH counterparts.The race
All four coins post
think()+clean_tracker()to a separatem_think_poolcompute thread.clean_tracker()'s exclusive prune (drop_tails) frees chain nodes on that thread. Two IO-thread sites in LTC touchedm_tracker.chainwithout holdingm_tracker_mutexfor the duration → the prune could free nodes mid-access → UAF SIGSEGV.The fix (LTC == BTC/BCH)
processing_shares_phase2— thestd::unique_lock(m_tracker_mutex, std::try_to_lock)is now declared at function scope and held across the whole mutation body;lock.unlock()only immediately before the asyncrun_think()post (the compute thread needs the exclusive lock). Adds theMAX_PENDING_ADDS(256) backpressure cap on the deferred queue, matching BTC/BCH.notify_local_share— the barem_tracker.chain.contains()pre-check is removed; both thecontains()read andattempt_verify()now run under the try-lock (lock.owns_lock() && chain.contains(...)). On lock-not-owned the inline verify is skipped — the share is already in-chain and the nextrun_think()cycle scores it.Adds
static constexpr size_t MAX_PENDING_ADDS = 256;tosrc/impl/ltc/node.hpp(already present in btc/bch).Consensus-neutrality
Lock-scope + guard only. Happy path is identical share processing; the lock-not-owned path queues/defers work that
think()drains next cycle anyway. Consensus diff-grep confirms every changed line is a lock primitive, a comment, the backpressure branch, or re-indentation of the existing defer-queue log lines. Normalized diffs of both functions vsimpl/btc/node.cppare empty.Test
Adds
Phase2HoldsLockAgainstComputePrunetotest/test_threading.cpp— a lock-discipline regression guard modelling the exact hazard (compute-thread exclusive prune freeing heap-allocated nodes while IO threads mutate/read under try-lock). A discipline break is a genuine UAF (fails under ASan/TSan). Existing threading tests unchanged.Scope
LTC only. Drift-map RANK-2 is the identical race in DGB (
impl/dgb/node.cpp); handled separately (DGB is not live-public and also lacks the think() watchdog — RANK-3).Prod status
LTC-DOGE test node was not actively crashing from this race at fix time (process ~3.6d uptime, no restart; last core was a SIGABRT, not the kr1z1s SIGSEGV). This is preventive hardening. Draft — no merge, no deploy (deploy is an operator tap; per-coin soak recommended first).
Refs #759.