highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-12-07 03:03:58 +01:00
Code Issues Packages Projects Releases Wiki Activity

net: fix race condition in self-connect detection

Browse Source 
Initiating an outbound network connection currently involves the
following steps after the socket connection is established (see
 `CConnman::OpenNetworkConnection` method):
    1. set up node state
    2. queue VERSION message
    3. add new node to vector `m_nodes`

If we connect to ourself, it can happen that the sent VERSION message
(step 2) is received and processed locally *before* the node object
is added to the connection manager's `m_nodes` vector (step 3). In this
case, the self-connect remains undiscovered, as the detection doesn't
find the outbound peer in `m_nodes` yet (see `CConnman::CheckIncomingNonce`).

Fix this by swapping the order of 2. and 3., by taking the `PushNodeVersion`
call out of `InitializeNode` and doing that in the `SendMessages` method
instead, which is only called for `CNode` instances in `m_nodes`.

Thanks go to vasild, mzumsande, dergoegge and sipa for suggestions on
how to fix this.
This commit is contained in:
Sebastian Falbesoner
2024-07-04 20:12:13 +02:00
parent 1c11089c7f
commit 66673f1c13
3 changed files with 16 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/net_processing.cpp
View File

@@ -243,6 +243,9 @@ struct Peer {
* Most peers use headers-first syncing, which doesn't use this mechanism */
uint256 m_continuation_block GUARDED_BY(m_block_inv_mutex) {};
/** Set to true once initial VERSION message was sent (only relevant for outbound peers). */
bool m_outbound_version_message_sent GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false};
/** This peer's reported block height when we connected */
std::atomic<int> m_starting_height{-1};
@@ -1677,9 +1680,6 @@ void PeerManagerImpl::InitializeNode(CNode& node, ServiceFlags our_services)
LOCK(m_peer_mutex);
m_peer_map.emplace_hint(m_peer_map.end(), nodeid, peer);
}
if (!node.IsInboundConn()) {
PushNodeVersion(node, *peer);
}
}
void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler)
@@ -5326,6 +5326,10 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt
PeerRef peer = GetPeerRef(pfrom->GetId());
if (peer == nullptr) return false;
// For outbound connections, ensure that the initial VERSION message
// has been sent first before processing any incoming messages
if (!pfrom->IsInboundConn() && !peer->m_outbound_version_message_sent) return false;
{
LOCK(peer->m_getdata_requests_mutex);
if (!peer->m_getdata_requests.empty()) {
@@ -5817,6 +5821,12 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
// disconnect misbehaving peers even before the version handshake is complete.
if (MaybeDiscourageAndDisconnect(*pto, *peer)) return true;
// Initiate version handshake for outbound connections
if (!pto->IsInboundConn() && !peer->m_outbound_version_message_sent) {
PushNodeVersion(*pto, *peer);
peer->m_outbound_version_message_sent = true;
}
// Don't send anything until the version handshake is complete
if (!pto->fSuccessfullyConnected || pto->fDisconnect)
return true;