From 7f36f122b2c25fac241946745e0764ac5847ce5b Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 23 Mar 2024 20:04:52 -0500 Subject: [PATCH 1/2] fix: fix potential mutex lifetime issue were we are returning a reference, and then releasing the mutex --- src/llmq/dkgsession.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index b819a59347..bacfafa0ed 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -663,8 +663,9 @@ void CDKGSession::VerifyAndJustify(CDKGPendingMessages& pendingMessages) continue; } - const auto& qc = WITH_LOCK(invCs, return std::move(complaints.at(*m->complaints.begin()))); - if (qc.complainForMembers[*myIdx]) { + LOCK(invCs); + if (const auto& qc = complaints.at(*m->complaints.begin()); + qc.complainForMembers[*myIdx]) { justifyFor.emplace(qc.proTxHash); } } From 6f2b350baa13e1c45c4b8ca5e4a805957b9d3984 Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 23 Mar 2024 20:27:00 -0500 Subject: [PATCH 2/2] fix: don't move out of pendingContributionVerifications; use a ref and then clear Moving out of a variable technically leaves it in an unspecified and potentially invalid state; yet I don't see anywhere we are reseting or otherwise clearing pendingContributionVerifications to ensure it is valid. Instead simply take a ref to it; and then clear it at the end. --- src/llmq/dkgsession.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index bacfafa0ed..5785fa1448 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -361,8 +361,7 @@ void CDKGSession::VerifyPendingContributions() cxxtimer::Timer t1(true); - std::vector pend = std::move(pendingContributionVerifications); - if (pend.empty()) { + if (pendingContributionVerifications.empty()) { return; } @@ -370,7 +369,7 @@ void CDKGSession::VerifyPendingContributions() std::vector vvecs; std::vector skContributions; - for (const auto& idx : pend) { + for (const auto& idx : pendingContributionVerifications) { const auto& m = members[idx]; if (m->bad || m->weComplain) { continue; @@ -404,7 +403,8 @@ void CDKGSession::VerifyPendingContributions() } } - logger.Batch("verified %d pending contributions. time=%d", pend.size(), t1.count()); + logger.Batch("verified %d pending contributions. time=%d", pendingContributionVerifications.size(), t1.count()); + pendingContributionVerifications.clear(); } void CDKGSession::VerifyAndComplain(CDKGPendingMessages& pendingMessages)