Skip to content

Commit 255004f

Browse files
committed
Merge bitcoin#29009: fuzz: p2p: Detect peer deadlocks
9f265d8 fuzz: Detect deadlocks in process_message (dergoegge) fae1e7e fuzz: p2p: Detect peer deadlocks (MarcoFalke) Pull request description: It may be possible that a peer connection will deadlock, due to software bugs such as bitcoin#18808. Fix this by detecting them in the fuzz target. Can be tested by introducing a bug such as: ```diff diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1067341..97495a13df 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2436,3 +2436,3 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic if (it != peer.m_getdata_requests.end() && !pfrom.fPauseSend) { - const CInv &inv = *it++; + const CInv& inv = *it; if (inv.IsGenBlkMsg()) { ``` Using a fuzz input such as: ``` $ base64 ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5 kNptdNbW1tbWYghvXIpwb25vPQAA////////cwAjLv8AXAB2ZXJhY2sAQW5v/62tra3Pz/////// //////////////////////9c8GZpbHRlcmxvYWQAAAEAAwAAAABVYwC2XABmaWx0ZXJhZGQAAAAX Fxdn/////2V0F861tcqvEmAAACEAAABjYXB0dXJldmUAAH4AgAA1PNfX11x0Z2V0ZGF0YQBDACOw AQMAAAAGIm5GERoLWcqvEmBD61u/KMNPOl4zKh/HKLK3PPGIkQ9eE/////////8AAAAAAAAAAFtb WyjDTzpeMSofx7K3PNfX11x0Z2V0ZGF0YQBDACMwAQMAAAAGIm5GERoLWcqvEmBD61u/KMNPOl4z Kh/Hsrc88YiRD2/Nzc3Nzc3Nzc3NTc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3N zWWj1NTUudTU1NTU1P///0j+P/9cdHR4AAAAAAAAy/4AAHR4AAAAAAAAP8v+AAD/+P////////// AX55bJl8HWnz/////wAgXGF0YVPxY2RkAAAA ``` And running the fuzz target: ``` $ FUZZ=process_messages ./src/test/fuzz/fuzz -runs=1 -timeout=18 ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5 INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 3436516708 INFO: Loaded 1 modules (390807 inline 8-bit counters): 390807 [0x55d0d6221e80, 0x55d0d6281517), INFO: Loaded 1 PC tables (390807 PCs): 390807 [0x55d0d6281518,0x55d0d6877e88), ./src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each. Running: ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5 ALARM: working on the last Unit for 19 seconds and the timeout value is 18 (use -timeout=N to change) ==375014== ERROR: libFuzzer: timeout after 19 seconds ``` ACKs for top commit: naumenkogs: ACK 9f265d8 dergoegge: ACK 9f265d8 brunoerg: ACK 9f265d8 Tree-SHA512: da83ff90962bb679aae00e8e9dba639c180b7aaba544e0c4d0978d36e28a9ff1cd7a2e13009d8ab407ef57767656aca1ebc767a7d2f1bc880284f8f57c197a50
2 parents 40bc501 + 9f265d8 commit 255004f

File tree

3 files changed

+28
-12
lines changed

3 files changed

+28
-12
lines changed

src/test/fuzz/process_message.cpp

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,23 @@ FUZZ_TARGET(process_message, .init = initialize_process_message)
7979
const auto mock_time = ConsumeTime(fuzzed_data_provider);
8080
SetMockTime(mock_time);
8181

82+
CSerializedNetMsg net_msg;
83+
net_msg.m_type = random_message_type;
8284
// fuzzed_data_provider is fully consumed after this call, don't use it
83-
DataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>()};
84-
try {
85-
g_setup->m_node.peerman->ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream,
86-
GetTime<std::chrono::microseconds>(), std::atomic<bool>{false});
87-
} catch (const std::ios_base::failure&) {
85+
net_msg.data = fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>();
86+
87+
connman.FlushSendBuffer(p2p_node);
88+
(void)connman.ReceiveMsgFrom(p2p_node, std::move(net_msg));
89+
90+
bool more_work{true};
91+
while (more_work) {
92+
p2p_node.fPauseSend = false;
93+
try {
94+
more_work = connman.ProcessMessagesOnce(p2p_node);
95+
} catch (const std::ios_base::failure&) {
96+
}
97+
g_setup->m_node.peerman->SendMessages(&p2p_node);
8898
}
89-
g_setup->m_node.peerman->SendMessages(&p2p_node);
9099
SyncWithValidationInterfaceQueue();
91100
g_setup->m_node.connman->StopNodes();
92101
}

src/test/fuzz/process_messages.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,17 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages)
7878

7979
connman.FlushSendBuffer(random_node);
8080
(void)connman.ReceiveMsgFrom(random_node, std::move(net_msg));
81-
random_node.fPauseSend = false;
8281

83-
try {
84-
connman.ProcessMessagesOnce(random_node);
85-
} catch (const std::ios_base::failure&) {
82+
bool more_work{true};
83+
while (more_work) { // Ensure that every message is eventually processed in some way or another
84+
random_node.fPauseSend = false;
85+
86+
try {
87+
more_work = connman.ProcessMessagesOnce(random_node);
88+
} catch (const std::ios_base::failure&) {
89+
}
90+
g_setup->m_node.peerman->SendMessages(&random_node);
8691
}
87-
g_setup->m_node.peerman->SendMessages(&random_node);
8892
}
8993
SyncWithValidationInterfaceQueue();
9094
g_setup->m_node.connman->StopNodes();

src/test/util/net.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,10 @@ struct ConnmanTestMsg : public CConnman {
7070
bool relay_txs)
7171
EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex);
7272

73-
void ProcessMessagesOnce(CNode& node) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex) { m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); }
73+
bool ProcessMessagesOnce(CNode& node) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex)
74+
{
75+
return m_msgproc->ProcessMessages(&node, flagInterruptMsgProc);
76+
}
7477

7578
void NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const;
7679

0 commit comments

Comments
 (0)