diff options
| author | fanquake <fanquake@gmail.com> | 2023-12-11 14:49:24 +0000 |
|---|---|---|
| committer | fanquake <fanquake@gmail.com> | 2023-12-11 15:05:40 +0000 |
| commit | 255004fc5e7febb1fbe2b0cdcbab3e0a5207acd9 (patch) | |
| tree | ad102c459b82ef402e09e2e322f03ba3d0aeb0d2 /src/test/fuzz | |
| parent | 40bc501bf462e1b38679f728336f18f08ee251ca (diff) | |
| parent | 9f265d88253ed464413dea5614fa13dea0d8cfd5 (diff) | |
Merge bitcoin/bitcoin#29009: fuzz: p2p: Detect peer deadlocks
9f265d88253ed464413dea5614fa13dea0d8cfd5 fuzz: Detect deadlocks in process_message (dergoegge)
fae1e7e012571201fd51c547ba4fb6044be9aeb5 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 https://github.com/bitcoin/bitcoin/pull/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 1067341495..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 9f265d88253ed464413dea5614fa13dea0d8cfd5
dergoegge:
ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5
brunoerg:
ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5
Tree-SHA512: da83ff90962bb679aae00e8e9dba639c180b7aaba544e0c4d0978d36e28a9ff1cd7a2e13009d8ab407ef57767656aca1ebc767a7d2f1bc880284f8f57c197a50
Diffstat (limited to 'src/test/fuzz')
| -rw-r--r-- | src/test/fuzz/process_message.cpp | 21 | ||||
| -rw-r--r-- | src/test/fuzz/process_messages.cpp | 14 |
2 files changed, 24 insertions, 11 deletions
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 6392f03d4e..acb03ac5fc 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -79,14 +79,23 @@ FUZZ_TARGET(process_message, .init = initialize_process_message) const auto mock_time = ConsumeTime(fuzzed_data_provider); SetMockTime(mock_time); + CSerializedNetMsg net_msg; + net_msg.m_type = random_message_type; // fuzzed_data_provider is fully consumed after this call, don't use it - DataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>()}; - try { - g_setup->m_node.peerman->ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream, - GetTime<std::chrono::microseconds>(), std::atomic<bool>{false}); - } catch (const std::ios_base::failure&) { + net_msg.data = fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(); + + connman.FlushSendBuffer(p2p_node); + (void)connman.ReceiveMsgFrom(p2p_node, std::move(net_msg)); + + bool more_work{true}; + while (more_work) { + p2p_node.fPauseSend = false; + try { + more_work = connman.ProcessMessagesOnce(p2p_node); + } catch (const std::ios_base::failure&) { + } + g_setup->m_node.peerman->SendMessages(&p2p_node); } - g_setup->m_node.peerman->SendMessages(&p2p_node); SyncWithValidationInterfaceQueue(); g_setup->m_node.connman->StopNodes(); } diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index b7e2e04b4b..3f722f60ee 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -78,13 +78,17 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages) connman.FlushSendBuffer(random_node); (void)connman.ReceiveMsgFrom(random_node, std::move(net_msg)); - random_node.fPauseSend = false; - try { - connman.ProcessMessagesOnce(random_node); - } catch (const std::ios_base::failure&) { + bool more_work{true}; + while (more_work) { // Ensure that every message is eventually processed in some way or another + random_node.fPauseSend = false; + + try { + more_work = connman.ProcessMessagesOnce(random_node); + } catch (const std::ios_base::failure&) { + } + g_setup->m_node.peerman->SendMessages(&random_node); } - g_setup->m_node.peerman->SendMessages(&random_node); } SyncWithValidationInterfaceQueue(); g_setup->m_node.connman->StopNodes(); |