IPC stability bugs

Bug 83

Reading a block template while submitting a solution for it is a data race

Status
Open, fix under review
Symptom
Data race; worst effect seen: sanitizer report only
Where seen
A fuzz target, Bitcoin Core unit tests
Trigger
Several calls running at the same time
First reported
Reported privately in an independent security review, 2026-09-06
Fixed
Not yet
Fix
bitcoin#36525
In released binaries
Yes (v30.0 onward)
Defect ever on master
Yes

Summary

A mining client gets a BlockTemplate from the node, reads it with getBlock, getBlockHeader and getCoinbaseMerklePath, and hands in a solved block with BlockTemplate.submitSolution. submitSolution writes the coinbase, header fields, merkle root and cached check flags into the template's stored block in place, with no lock, while the getters read the same block. An IPC client that uses one template from two threads (each client thread gets its own worker thread on the server) can make these run at the same time: a data race in bitcoin-node. Most fields would only be read torn, but the coinbase std::shared_ptr (vtx[0]) is copied on one thread while it is reassigned on another, which can corrupt its reference count. It was reported privately in an independent security review with an in-process test that ThreadSanitizer flags, and a separate test gives the same ThreadSanitizer reports; it was later reported publicly in bitcoin#36515, found with the fuzzamoto fuzzer under ThreadSanitizer over real IPC. It has not been seen without the sanitizer. Released binaries from v30.0 on are affected. A fix is in review in bitcoin#36525.

What happened

An independent security review of Bitcoin Core's IPC code reported the race privately (2026-09-06). The report includes an in-process Boost test that calls getBlock, getBlockHeader and getCoinbaseMerklePath on one thread while submitSolution runs on another; under ThreadSanitizer, without the proposed fix, it reports the conflicting access. The report does not claim a corrupted template, an invalid submission or a crash, and describes the test as not an end-to-end IPC demonstration. The first public report is bitcoin#36515 (2026-10-13): running the fuzzamoto fuzzer against a ThreadSanitizer build of bitcoin-node, with getBlockHeader on one IPC worker thread and submitSolution on another, gave the same data race over real IPC.

Root cause

BlockTemplateImpl (src/node/interfaces.cpp) holds one CBlockTemplate and has no mutex. submitSolution calls AddMerkleRootAndCoinbase(m_block_template->block, ...) (src/node/miner.cpp), which writes the stored block: block.vtx[0] = coinbase, nVersion, nTime, nNonce, hashMerkleRoot, and the m_checked_witness_commitment, m_checked_merkle_root and fChecked flags. It then copies the block for submission. getBlockHeader, getBlock and getCoinbaseMerklePath read the same CBlock, as does a second submitSolution on the same template.

The code assumes a template is used by one caller at a time. That was harmless while submitSolution only read the template. waitNext is not affected: WaitAndCreateNewBlock reads only hashPrevBlock and vTxFees, which submitSolution does not write.

Multi-threaded clients that share a template exist: in bitcoin#33923, an issue asking whether concurrent IPC requests on the same thread should crash the node, @plebhash described a Rust Stratum v2 client that runs submitSolution, the getters and waitNext on separate server threads. Whether it ever overlaps a getter and submitSolution on the same template is not known.

Fix

A fix is in review: bitcoin#36525 (opened 2026-10-14) protects m_block_template with a mutex, in its last commit; its author will open a standalone fix if that PR stalls. The private report proposes a mutex around template access, with waitNext taking a snapshot of hashPrevBlock and vTxFees before it waits.

Reproduction

A new miner_tests case in test_bitcoin creates one block template, then calls getBlock() 200 times on one thread while the main thread calls submitSolution 200 times with fresh coinbases on the same template. Over IPC the two calls would run on two server worker threads, which is the same concurrency. It was run on 2026-09-26 against Bitcoin Core master bfdcd97, which carries libmultiprocess v13 in its subtree.

Patch: bitcoin-submitsolution-race-unit.patch. It applies to Bitcoin Core master; last checked against badc957 on 2026-10-15. From a Bitcoin Core checkout:

curl -sO https://ipcbugs.ofsky.org/repro/bitcoin-submitsolution-race-unit.patch
git apply bitcoin-submitsolution-race-unit.patch
cmake -B build-tsan -DCMAKE_BUILD_TYPE=Debug -DSANITIZERS=thread && cmake --build build-tsan --target test_bitcoin
build-tsan/bin/test_bitcoin --run_test=miner_tests/repro_submitsolution_concurrent_getblock

ThreadSanitizer reported 4 data races. The first is the coinbase shared_ptr assignment against the copy of the block in getBlock(); the others cover the header fields (miner.cpp:362):

WARNING: ThreadSanitizer: data race
  Write of size 8 ... by main thread:
    #1 std::shared_ptr<CTransaction const>::operator=(...)
    #2 node::AddMerkleRootAndCoinbase(...) src/node/miner.cpp:354:22
  Previous read ... by thread T6:
    #1 CBlock::CBlock(CBlock const&) src/primitives/block.h:73:7
    #2 node::(anonymous namespace)::BlockTemplateImpl::getBlock() src/node/interfaces.cpp:898:16
ThreadSanitizer: reported 4 warnings

The same test passes in a plain Debug build: no torn result or crash was seen without the sanitizer. The test patch is available on request and will be included in the GitHub issue for this bug when it is opened.

Next steps

Timeline

Lessons

What would have prevented this bug, found it sooner, or made it easier to debug. The patterns page groups these lessons across all bugs.

Pattern A contract changed and a caller was left behind: A function's contract changed under its callers

  • Prevented by the type system, a review checklist item
  • Earliest detector ThreadSanitizer (did not exist then)
  • Diagnosis easy
  • Fix design easy
  • Debugging aids sanitizer output
  • Rust yes

Would Rust have prevented it? Yes. A template shared across server threads would be an Arc<BlockTemplate>, and submitSolution's in-place write through it would not compile without a Mutex, so the refactor from copy to in-place mutation would have been rejected at the call site. More on Rust

Could it have been prevented?

Yes. From bitcoin#30955 until May 2025 submitSolution copied the template's block (CBlock block{m_block_template->block}) and changed the copy, so a BlockTemplate was immutable after creation and safe to read from any number of threads. Commit e6c2f4ce7a84, titled a refactor, dropped the copy and passed the stored block to AddMerkleRootAndCoinbase(CBlock&, ...). Had BlockTemplateImpl held the template through a const pointer to const data, the refactor would not have compiled; that is the cheapest guard for an object handed out over IPC, where every client thread is its own server thread and no method can assume it is the only caller. The review item is "does a refactor turn a copy into an in-place write on shared state", which is answerable from the diff.

Could it have been found sooner?

Yes, under ThreadSanitizer, but only with a test that reads and submits on one template from two threads. Bitcoin Core's TSan CI job existed throughout; miner_tests and the IPC functional tests use a template from one thread at a time, so there was nothing for it to see. The independent security review's in-process Boost test shows the race on its first TSan run, and the survey's own miner_tests case did the same against master (4 reports, the first on the vtx[0] shared_ptr). A Stratum v2 client that uses one template from several threads exists (bitcoin#33923), but a torn header or a corrupted refcount would show as a rejected share or a rare crash with no link back to this cause, so the field was unlikely to report it.

What made it hard to debug?

Nothing once looked for: TSan names both accesses and the two lines. The difficulty is noticing it at all, because the in-place write was introduced as a behavior-neutral refactor, and bitcoin#33745 later added more in-place writes (the cached check flags) on the same assumption without anyone asking who else reads the block. Both changes were reviewed as mining logic, not as changes to an object's thread-safety contract.

Lesson

An object returned to remote callers is a shared object; keep it immutable or lock it, and treat a refactor that replaces "modify a copy" with "modify in place" as a change of contract rather than a cleanup.

Notes

Classification

The survey's classification of this bug. The explorer charts these fields across all bugs; the method page explains them.

Field Value
Symptom data-race
Worst effect observed sanitizer-report-only
Seen in fuzz-target, unit-test
Scope production
Trigger Concurrent calls (concurrent-calls)
Phase steady-state
Side server
Component bitcoin-mining
Root cause kind missing-lock
Fix kind none
Detection review
Needs a peer that is not libmultiprocess C++ no
Needed a sanitizer to see yes
Defect ever on master yes
Released binary
Confidence medium