From 0b6c7d62135cc52974e851ea4f8263a3a7b6cb33 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 22 Nov 2020 18:33:21 +0300 Subject: [PATCH 1/5] Fix potential deadlock in `CSporkManager::UpdateSpork()` --- src/spork.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/spork.cpp b/src/spork.cpp index 0cd515ebba2a3..9bff991fa761e 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -186,8 +186,6 @@ bool CSporkManager::UpdateSpork(SporkId nSporkID, int64_t nValue, CConnman& conn { CSporkMessage spork = CSporkMessage(nSporkID, nValue, GetAdjustedTime()); - LOCK(cs); - if (!spork.Sign(sporkPrivKey)) { LogPrintf("CSporkManager::%s -- ERROR: signing failed for spork %d\n", __func__, nSporkID); return false; @@ -201,8 +199,11 @@ bool CSporkManager::UpdateSpork(SporkId nSporkID, int64_t nValue, CConnman& conn LogPrintf("CSporkManager::%s -- signed %d %s\n", __func__, nSporkID, spork.GetHash().ToString()); - mapSporksByHash[spork.GetHash()] = spork; - mapSporksActive[nSporkID][keyIDSigner] = spork; + { + LOCK(cs); + mapSporksByHash[spork.GetHash()] = spork; + mapSporksActive[nSporkID][keyIDSigner] = spork; + } spork.Relay(connman); return true; From 5c44ccdd0371e553c83a4cc787e3b6f221b488ef Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 26 Nov 2020 13:00:42 +0300 Subject: [PATCH 2/5] Protect `inputRequestIds` with cs lock --- src/llmq/quorums_instantsend.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 63a8b1b229d06..acd87816ad410 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -458,7 +458,10 @@ bool CInstantSendManager::ProcessTx(const CTransaction& tx, bool allowReSigning, for (size_t i = 0; i < tx.vin.size(); i++) { auto& in = tx.vin[i]; auto& id = ids[i]; - inputRequestIds.emplace(id); + { + LOCK(cs); + inputRequestIds.emplace(id); + } LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s: trying to vote on input %s with id %s. allowReSigning=%d\n", __func__, tx.GetHash().ToString(), in.prevout.ToStringShort(), id.ToString(), allowReSigning); if (quorumSigningManager->AsyncSignIfMember(llmqType, id, tx.GetHash(), allowReSigning)) { @@ -1119,6 +1122,7 @@ void CInstantSendManager::RemoveConflictedTx(const CTransaction& tx) void CInstantSendManager::TruncateRecoveredSigsForInputs(const llmq::CInstantSendLock& islock) { + AssertLockHeld(cs); auto& consensusParams = Params().GetConsensus(); for (auto& in : islock.inputs) { From f57e6662b0eb0206499f4d8e8a40b7cdb4e38b61 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 26 Nov 2020 17:25:31 +0300 Subject: [PATCH 3/5] Protect `curDBTransaction` in `CEvoDB::CommitRootTransaction()` --- src/evo/evodb.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/evo/evodb.cpp b/src/evo/evodb.cpp index 5070ec38a1b57..dc20645fdcb4b 100644 --- a/src/evo/evodb.cpp +++ b/src/evo/evodb.cpp @@ -53,6 +53,7 @@ void CEvoDB::RollbackCurTransaction() bool CEvoDB::CommitRootTransaction() { + LOCK(cs); assert(curDBTransaction.IsClean()); rootDBTransaction.Commit(); bool ret = db.WriteBatch(rootBatch); From 015a39436772d2e5dfd35a3d7d876561e5dde4dd Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 26 Nov 2020 18:15:42 +0300 Subject: [PATCH 4/5] Check for `AssertLockNotHeld` in `EnforceBestChainLock()` instead of just having a comment in code --- src/llmq/quorums_chainlocks.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index 06e717450d0c9..6a36a73b4eaa9 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -488,6 +488,9 @@ bool CChainLocksHandler::IsTxSafeForMining(const uint256& txid) // This should also not be called from validation signals, as this might result in recursive calls void CChainLocksHandler::EnforceBestChainLock() { + AssertLockNotHeld(cs); + AssertLockNotHeld(cs_main); + CChainLockSig clsig; const CBlockIndex* pindex; const CBlockIndex* currentBestChainLockBlockIndex; From 9111fea999be73041fe37e8c8a94f94aae9927f4 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 27 Nov 2020 02:20:11 +0300 Subject: [PATCH 5/5] Protect spork maps on (de)serialization --- src/spork.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/spork.h b/src/spork.h index ea490688c1cd7..53079e534525a 100644 --- a/src/spork.h +++ b/src/spork.h @@ -191,6 +191,7 @@ class CSporkManager // we don't serialize pubkey ids because pubkeys should be // hardcoded or be setted with cmdline or options, should // not reuse pubkeys from previous dashd run + LOCK(cs); READWRITE(mapSporksByHash); READWRITE(mapSporksActive); // we don't serialize private key to prevent its leakage