What the developer wrote
Authored by tobtoht
Merge pull request #11321
46807c4 blockchain: fix lock ordering in check_against_checkpoints (selsta)
ACKs: jpk68, SNeedlewoods
What changed, and why it matters
This patch changes the order in which two internal locks are acquired in a Monero blockchain-checking function. In multi-threaded software, taking locks in the wrong order can cause a 'deadlock,' where two parts of the program wait forever for each other and the node can freeze. The change makes the lock order consistent with the rest of the codebase, which is a defensive fix. There is no direct evidence in the commit that an attacker can trigger the deadlock on demand, but lock-order bugs are a classic source of denial-of-service problems in network-facing daemons.
Review the global lock hierarchy for m_tx_pool and m_blockchain_lock across the whole repository to confirm this is the only inconsistent ordering. Run static lock-order analyzers and stress-test the node under concurrent checkpoint validation and transaction-pool operations. Consider this for a routine security maintenance release.
Security signals we found
Lock-order change in concurrent code
Potential deadlock (AB-BA) between m_tx_pool and m_blockchain_lock
Denial-of-service risk if a node can be frozen
No input validation or memory-safety bug visible
Evidence from the diff
In Blockchain::check_against_checkpoints(), the patch now acquires m_tx_pool before m_blockchain_lock (using CRITICAL_REGION_LOCAL then CRITICAL_REGION_LOCAL1), whereas previously only m_blockchain_lock was held. This aligns the lock ordering with other functions in the same file that take both locks in the same sequence. The change prevents potential AB-BA deadlocks when one thread holds m_blockchain_lock and waits for m_tx_pool while another thread holds m_tx_pool and waits for m_blockchain_lock. The patch is small and partial: it fixes one site but does not prove the entire lock hierarchy is now consistent.
Changed components
src/cryptonote_core/blockchain.cppBlockchain::check_against_checkpoints()m_tx_pool mutexm_blockchain_lock mutexInspect captured patch +2 / −1
### src/cryptonote_core/blockchain.cpp
@@ -4557,7 +4557,8 @@ void Blockchain::check_against_checkpoints(const checkpoints& points, bool enfor
const auto& pts = points.get_points();
bool stop_batch;
- CRITICAL_REGION_LOCAL(m_blockchain_lock);
+ CRITICAL_REGION_LOCAL(m_tx_pool);
+ CRITICAL_REGION_LOCAL1(m_blockchain_lock);
stop_batch = m_db->batch_start();
const uint64_t blockchain_height = m_db->height();
for (const auto& pt : pts)Why this scored 46/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.