net: avoid recursive m_nodes_mutex lock in DisconnectNode()
What the developer wrote
Authored by Vasil Dimov
net: avoid recursive m_nodes_mutex lock in DisconnectNode()
Have `CConnman::DisconnectNode()` iterate `m_nodes` itself instead of
using `FindNode()`. This avoids recursive mutex lock and drops the only
caller of `FindNode()` which used the return value for something else
than a boolean found/notfound.
What changed, and why it matters
This commit changes how Bitcoin Core finds a network peer when disconnecting it by address. Previously it called an internal helper (FindNode) that would lock the same peer-list mutex again while already holding it, creating a recursive lock. The patch makes the function search the peer list directly, avoiding that double-lock. Recursive mutex locks can hide deadlocks or, in some lock implementations, cause failures if the mutex is later changed to non-recursive. The change is defensive cleanup rather than a fix for a known remotely exploitable bug.
Treat as low-priority defensive maintenance. Review whether other recursive locks on m_nodes_mutex remain and consider documenting the lock's expected non-recursive semantics. No emergency deployment is warranted based on this diff alone.
Security signals we found
Recursive mutex lock pattern removed
Peer-list mutex (m_nodes_mutex) locking simplified
No functional change to disconnection logic
Defensive hardening against future non-recursive mutex implementation
Evidence from the diff
CConnman::DisconnectNode(const std::string&) previously called FindNode() while already holding m_nodes_mutex. FindNode internally takes the same mutex, producing a recursive lock. The patch replaces the FindNode call with a std::ranges::find_if over m_nodes comparing node->m_addr_name to strNode, keeping the existing single LOCK(m_nodes_mutex). This removes the only caller that used FindNode’s returned CNode* for more than a boolean check. The functional behavior is unchanged; the benefit is eliminating a recursive lock pattern that could become unsafe if m_nodes_mutex is ever made non-recursive or if lock-order reasoning is needed.
Changed components
src/net.cppCConnman::DisconnectNode(const std::string&)m_nodes_mutexFindNode helperInspect captured patch +5 / −3
diff --git a/src/net.cpp b/src/net.cpp
index e8d2819a..6e95d6a3 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -3631,9 +3631,11 @@ void CConnman::GetNodeStats(std::vector<CNodeStats>& vstats) const
bool CConnman::DisconnectNode(const std::string& strNode)
{
LOCK(m_nodes_mutex);
- if (CNode* pnode = FindNode(strNode)) {
- LogDebug(BCLog::NET, "disconnect by address%s match, %s", (fLogIPs ? strprintf("=%s", strNode) : ""), pnode->DisconnectMsg(fLogIPs));
- pnode->fDisconnect = true;
+ auto it = std::ranges::find_if(m_nodes, [&strNode](CNode* node) { return node->m_addr_name == strNode; });
+ if (it != m_nodes.end()) {
+ CNode* node{*it};
+ LogDebug(BCLog::NET, "disconnect by address%s match, %s", (fLogIPs ? strprintf("=%s", strNode) : ""), node->DisconnectMsg(fLogIPs));
+ node->fDisconnect = true;
return true;
}
return false;
Why this scored 23/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.