Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fuzz: p2p: Detect peer deadlocks #29009

Merged
merged 2 commits into from Dec 11, 2023
Merged

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Dec 6, 2023

It may be possible that a peer connection will deadlock, due to software bugs such as #18808.

Fix this by detecting them in the fuzz target.

Can be tested by introducing a bug such as:

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

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 6, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK dergoegge, naumenkogs, brunoerg
Concept ACK furszy

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #28960 (kernel: Remove dependency on CScheduler by TheCharlatan)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot added the Tests label Dec 6, 2023
Copy link
Member

@furszy furszy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

@dergoegge
Copy link
Member

Should we do the same for process_message? would need to change it to use ProcessMessagesOnce as well.

@maflcko
Copy link
Member Author

maflcko commented Dec 6, 2023

Should we do the same for process_message? would need to change it to use ProcessMessagesOnce as well.

Happy to review a pull request, or happy to include any patch here, that compiles, if someone writes it.

@dergoegge
Copy link
Member

Happy to review a pull request, or happy to include any patch here, that compiles, if someone writes it.

feel free to pick dergoegge@9f265d8

@brunoerg
Copy link
Contributor

brunoerg commented Dec 6, 2023

Concept ACK

@naumenkogs
Copy link
Member

Concept ACK

Copy link
Member

@dergoegge dergoegge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 9f265d8

@fanquake
Copy link
Member

fanquake commented Dec 8, 2023

cc also @mzumsande @sipa

@naumenkogs
Copy link
Member

ACK 9f265d8

@DrahtBot DrahtBot removed the request for review from naumenkogs December 11, 2023 09:37
Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 9f265d8

@fanquake fanquake merged commit 255004f into bitcoin:master Dec 11, 2023
19 checks passed
@maflcko maflcko deleted the 2312-fuzz-p2p-dead- branch December 11, 2023 16:44
Copy link
Member

@jamesob jamesob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 9f265d8 (jamesob/ackr/29009.1.maflcko.fuzz_p2p_detect_peer_dea)

Using the suggested buggy diff, I was able to reproduce a fuzz timeout locally:

SUMMARY: libFuzzer: timeout
================== Job 4 exited with exit code 70 ============
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3004856548
INFO: Loaded 1 modules   (549836 inline 8-bit counters): 549836 [0x55b1d6e36508, 0x55b1d6ebc8d4),
INFO: Loaded 1 PC tables (549836 PCs): 549836 [0x55b1d6ebc8d8,0x55b1d7720598),
INFO:     2124 files found in ../qa-assets/fuzz_seed_corpus/process_message
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1047633 bytes
INFO: seed corpus: files: 2124 min: 1b max: 1047633b total: 78093906b rss: 247Mb
Warning: Please check that your computer's date and time are correct! If your clock is wrong, Bitcoin Core will not work properly.
ALARM: working on the last Unit for 1796 seconds
       and the timeout value is 1200 (use -timeout=N to change)
MS: 0 ; base unit: 0000000000000000000000000000000000000000
artifact_prefix='./'; Test unit written to ./timeout-3a21200cd77fa45164203d47b65985b623d18c8f
==1628346== ERROR: libFuzzer: timeout after 1796 seconds
    #0 0x55b1d2def21f  (/home/james/src/bitcoin/src/test/fuzz/fuzz+0x1db721f) (BuildId: 6352bec346d40adbe5b31bd18e73b640a40465c2)

    [ ... snip ... ]

    #17 0x7f8844188ccf  (/usr/lib/libc.so.6+0x27ccf) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)
    #18 0x7f8844188d89  (/usr/lib/libc.so.6+0x27d89) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)
    #19 0x55b1d2c9e304  (/home/james/src/bitcoin/src/test/fuzz/fuzz+0x1c66304) (BuildId: 6352bec346d40adbe5b31bd18e73b640a40465c2)

SUMMARY: libFuzzer: timeout
FUZZ=process_message ./src/test/fuzz/fuzz  -rss_limit_mb=20000 -jobs=12  7716.06s user 734.55s system 463% cpu 30:21.38 total

[1] 15:38:59 james@fido src/bitcoin (?± ackr/29009.1.maflcko.fuzz_p2p_detect_peer_dea 9f265d8) % base64 ./timeout-3a21200cd77fa45164203d47b65985b623d18c8f

Z2V0ZGF0YQALkmNvbm7///r/+v///////////////2dldGEA//+JHgoBAABAAP//QTBloTBlMDp0
imRhdGEA/2VkdGFndGEA//+JHgoBAABAAAEwMP9lZ0FlMDp0imRhlZ4A/2VkdGFndGEA//+JHgIB
AABAAP9BKDAwMGV0ZGfL+SybqZYAFfABduEAD2dldGRhdGEBAABAZXRkYXRhAGP/c2VuZGFkZHJ2
MjB24f0BbH8AAGf///9x//////////8BAABA/x4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4e
Hh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4e////
//////////8D/+r/////BAIA//9B/9//MDAyMTM4NDAzMzYwZ2V0ZGF0YQB0AHRgBAD//4keAgD/
/2dljJthdGEA/////0H/AAAAAbNc/evr//8PAOvrrP///2VnbpYAFWH9////AQAAAAAAAAABZYsA
lP5kYXRhAGP/bH8DMDU1AGRhdH//WAj/ZAJldGRhYQB0dGD/f/94CIODg4ODg4OD////////////
1f////////8AAAAAAAAAExMBAAAAAAAAAAAAAAAAhAAAAAAAAAAAAAAAAAAAAAAAOgAAAAD+//8a
AAAAZ2V0ZAAAAAAAAAB0YQ==
Show signature data

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5 ([`jamesob/ackr/29009.1.maflcko.fuzz_p2p_detect_peer_dea`](https://github.com/jamesob/bitcoin/tree/ackr/29009.1.maflcko.fuzz_p2p_detect_peer_dea))

Using the suggested buggy diff, I was able to reproduce a fuzz timeout locally:

SUMMARY: libFuzzer: timeout
================== Job 4 exited with exit code 70 ============
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3004856548
INFO: Loaded 1 modules (549836 inline 8-bit counters): 549836 [0x55b1d6e36508, 0x55b1d6ebc8d4),
INFO: Loaded 1 PC tables (549836 PCs): 549836 [0x55b1d6ebc8d8,0x55b1d7720598),
INFO: 2124 files found in ../qa-assets/fuzz_seed_corpus/process_message
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1047633 bytes
INFO: seed corpus: files: 2124 min: 1b max: 1047633b total: 78093906b rss: 247Mb
Warning: Please check that your computer's date and time are correct! If your clock is wrong, Bitcoin Core will not work properly.
ALARM: working on the last Unit for 1796 seconds
and the timeout value is 1200 (use -timeout=N to change)
MS: 0 ; base unit: 0000000000000000000000000000000000000000
artifact_prefix='./'; Test unit written to ./timeout-3a21200cd77fa45164203d47b65985b623d18c8f
==1628346== ERROR: libFuzzer: timeout after 1796 seconds
#0 0x55b1d2def21f (/home/james/src/bitcoin/src/test/fuzz/fuzz+0x1db721f) (BuildId: 6352bec346d40adbe5b31bd18e73b640a40465c2)

[ ... snip ... ]

#17 0x7f8844188ccf  (/usr/lib/libc.so.6+0x27ccf) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)
#18 0x7f8844188d89  (/usr/lib/libc.so.6+0x27d89) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)
#19 0x55b1d2c9e304  (/home/james/src/bitcoin/src/test/fuzz/fuzz+0x1c66304) (BuildId: 6352bec346d40adbe5b31bd18e73b640a40465c2)

SUMMARY: libFuzzer: timeout
FUZZ=process_message ./src/test/fuzz/fuzz -rss_limit_mb=20000 -jobs=12 7716.06s user 734.55s system 463% cpu 30:21.38 total

[1] 15:38:59 james@fido src/bitcoin (?± ackr/29009.1.maflcko.fuzz_p2p_detect_peer_dea 9f265d8) % base64 ./timeout-3a21200cd77fa45164203d47b65985b623d18c8f

Z2V0ZGF0YQALkmNvbm7///r/+v///////////////2dldGEA//+JHgoBAABAAP//QTBloTBlMDp0
imRhdGEA/2VkdGFndGEA//+JHgoBAABAAAEwMP9lZ0FlMDp0imRhlZ4A/2VkdGFndGEA//+JHgIB
AABAAP9BKDAwMGV0ZGfL+SybqZYAFfABduEAD2dldGRhdGEBAABAZXRkYXRhAGP/c2VuZGFkZHJ2
MjB24f0BbH8AAGf///9x//////////8BAABA/x4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4e
Hh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4e////
//////////8D/+r/////BAIA//9B/9//MDAyMTM4NDAzMzYwZ2V0ZGF0YQB0AHRgBAD//4keAgD/
/2dljJthdGEA/////0H/AAAAAbNc/evr//8PAOvrrP///2VnbpYAFWH9////AQAAAAAAAAABZYsA
lP5kYXRhAGP/bH8DMDU1AGRhdH//WAj/ZAJldGRhYQB0dGD/f/94CIODg4ODg4OD////////////
1f////////8AAAAAAAAAExMBAAAAAAAAAAAAAAAAhAAAAAAAAAAAAAAAAAAAAAAAOgAAAAD+//8a
AAAAZ2V0ZAAAAAAAAAB0YQ==



-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEGNRVI1NPYuZCSIrGepNdrbLETwUFAmV4d6oACgkQepNdrbLE
TwXLkBAAk/7T4AcyzzjRrAaKvF+wShsYO+MA/FbLY6OuwehUlKemRFCj3/CoYr25
u/K955m/RA9VOR/OZZH3lohNZMhA01yEbaIWzvYe2ZBeXvTNOBEQgPr61WBmY6tC
wTXRBa5kWwAc5JMfksKdvxgDrVRRQalwoqSS2r1fmfNVqF5zmgYwSVYClJuHfwuO
tTrZFYZwGOdpgRSHux2os51oGep7TJhA+xxcM0ZunsPHqCWm6DD7JiVJhoNk+qG+
YJSjXUJjH9qM9VJ8xOziEtESDGiMCHY5XVYzw6jhYwjUnk/NdGUsAWdM6x1mUx35
+wus5szjxkyb5ABEGtPggJSiEBxCoZ3i6fTzT3kK4QNqalp+Qho/3Q6oDLEhVnuk
q1440aw9BR+hQbDdlZjWxSjctAzF7b0BP4Km0VBYZjKbzgpLUKgUqo3rGCF89MZj
c9BoyY9FXIYKhFBhs0DMSArvpjH8B7h5e7HBMYxRQSTT6kldYwKwxJkXohOO5fvO
tDX0rQIFqP+vdQpBtNxxRVpkOsLof39quUVZwIFGfcf2OgwzdVn/vo70kR1IfUjh
LonnxfA37AQl8/PwI4Zztet2nna5Ao2BceB3xfzhuvkb8UH/H/MwSAkcH9yFoyUU
YvssqGIqGejYhMSNOMAfX57QI3WHAXHmTFEr/qcNJVtKvYIWgIg=
=5BSl
-----END PGP SIGNATURE-----

Show platform data

Tested on Linux-6.5.5-arch1-1-x86_64-with-glibc2.38

Configured with ./configure 'BDB_LIBS=-L/home/james/src/bitcoin/db4/lib -ldb_cxx-4.8' BDB_CFLAGS=-I/home/james/src/bitcoin/db4/include 'CXXFLAGS=-fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare ' --disable-gui-tests --enable-fuzz --with-sanitizers=address,fuzzer,undefined --enable-wallet --enable-debug --with-daemon --enable-natpmp-default

Compiled with /usr/bin/ccache /usr/bin/clang++ -std=c++17 -mavx -mavx2 -mpclmul -fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare  -O0 -g3 -ftrapv -fdebug-prefix-map=$(abs_top_srcdir)=.  -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -fsanitize=address,fuzzer,undefined -msse4.1 -msse4.2 -msse4 -msha  i

Compiler version: clang version 16.0.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants