Extend CConnman fuzz tests to also exercise the methods OpenNetworkConnection(), CreateNodeFromAcceptedSocket(), InitBinds() and SocketHandler().
Previously fuzzing those methods would have resulted in real socket functions being called in the operating system which is undesirable during fuzzing. Now that #21878 is complete all those are mocked to a fuzzed socket and a fuzzed DNS resolver (see how CreateSock and g_dns_lookup are replaced in the first commit).
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/28584.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
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.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
Concept ACK
The cpu utilization of this target doesn't look great, i suspect this is due to timeouts/sleeps e.g.: https://github.com/bitcoin/bitcoin/blob/db7b5dfcc502a8a81c51f56fe753990ae8b3a202/src/net.cpp#L2099
Would be nice to address that (maybe in a follow up).
Here is a preliminary coverage report: https://dergoegge.github.io/bitcoin-coverage/pr28584/fuzz.coverage/index.html (I'll update this after fuzzing for longer). Looks good to me as the target alone now has more coverage (by line count) in net.cpp than our coverage on master with all targets.
Concept ACK
@dergoegge, thanks for looking into this!
The cpu utilization of this target doesn't look great
How did you measure? I guess it should be possible to mock CConnman::interruptNet so that its sleep_for() method returns immediately.
How did you measure?
Eyeballing htop this target only achieves about 10% - 30% per core on my machine.
I guess it should be possible to mock CConnman::interruptNet so that its sleep_for() method returns immediately.
Sounds good to me.
When I run FUZZ=connman ./src/test/fuzz/fuzz it is single CPU/single threaded and it stays at 98%-100% CPU all the time. It is the same in master. Does it get executed on >1 cores for you?
When I run FUZZ=connman ./src/test/fuzz/fuzz it is single CPU/single threaded and it stays at 98%-100% CPU all the time.
For how long did you run this? The fuzzer needs to first find inputs that trigger sleep_for for you to be able to observe the problem. I have tested this on two machines now and the result is always the same.
Does it get executed on >1 cores for you?
You can let libfuzzer run on multiple cores with either -jobs=<num cpus> or -fork=<num cpus>. I prefer fork since it also includes a minimization step.
20a9fc83bd...cd5bbb12e0: rebase due to conflicts
159 | @@ -160,6 +160,15 @@ FUZZ_TARGET(connman, .init = initialize_connman) 160 | /*strDest=*/fuzzed_data_provider.ConsumeBool() ? nullptr : random_string.c_str(), 161 | /*conn_type=*/conn_type, 162 | /*use_v2transport=*/fuzzed_data_provider.ConsumeBool()); 163 | + }, 164 | + [&] { 165 | + connman.SetNetworkActive(true);
In b1b74a13adb8e943d90fbe56c4a706b0ae91335a: nit: I don't think to set network active here is a must, we could fuzz it with both network active and inactive.
Right, done!
100 | +{ 101 | + std::vector<CService> ret; 102 | + const size_t size = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, max_vector_size); 103 | + ret.reserve(size); 104 | + for (size_t i = 0; i < size; ++i) { 105 | + ret.emplace_back(ConsumeNetAddr(fuzzed_data_provider),
In d923b86264df93ad86c7bd557b9970e156585dc7: You could use ConsumeService into ConsumeServiceVector.
diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h
index a97017555d..78e61b51d9 100644
--- a/src/test/fuzz/util/net.h
+++ b/src/test/fuzz/util/net.h
@@ -102,8 +102,7 @@ inline std::vector<CService> ConsumeServiceVector(FuzzedDataProvider& fuzzed_dat
const size_t size = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, max_vector_size);
ret.reserve(size);
for (size_t i = 0; i < size; ++i) {
- ret.emplace_back(ConsumeNetAddr(fuzzed_data_provider),
- fuzzed_data_provider.ConsumeIntegral<uint16_t>());
+ ret.emplace_back(ConsumeService(fuzzed_data_provider));
}
return ret;
}
Done, thanks!
cd5bbb12e0...5e3c80da14: address suggestions
utACK 5e3c80da1473f90406b84c1ba14dc563ce4d2853
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.
Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
Leave a comment here, if you need help tracking down a confusing failure.
<sub>Debug: https://github.com/bitcoin/bitcoin/runs/21199329709</sub>
5e3c80da14...6d9083b249: rebase due to silent merge conflict
reACK 6d9083b249376d503621da7980ef7ae02e690e0b
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.
Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
Leave a comment here, if you need help tracking down a confusing failure.
<sub>Debug: https://github.com/bitcoin/bitcoin/runs/23166298024</sub>
6d9083b249...45f4dbe484: rebase due to conflicts
45f4dbe484...655a2cf666: the previous push resolved the merge conflict in a too late commit, causing the "test each commit" CI job to fail
655a2cf666...99b1f88fe8: rebase to pick CMake
99b1f88fe8...cf83f0c14c: rebase due to conflicts and mock the sleeps
I guess it should be possible to mock CConnman::interruptNet so that its sleep_for() method returns immediately.
Sounds good to me.
Done. CThreadInterrupt can now be mocked in other tests as well. Thanks for the suggestion, @dergoegge!
14 | + return m_fuzzed_data_provider.ConsumeBool(); 15 | +} 16 | + 17 | +bool FuzzedThreadInterrupt::sleep_for(Clock::duration) 18 | +{ 19 | + return m_fuzzed_data_provider.ConsumeBool();
Perhaps in a follow up we can make mocktime accurate to the millisecond and then advance it here to simulate an actual sleep.
I like simulating some time passage here.
Changing static std::atomic<std::chrono::seconds> g_mock_time{}; to a finer precision is indeed out of the scope here.
Following a discussion on IRC I checked that indeed the time is frozen already for fuzz tests so calling SetMockTime() here is not going to freeze it (since it is already frozen). I added:
SetMockTime(ConsumeTime(m_fuzzed_data_provider)); // Time could go backwards.
It could end up increasing the time more than the argument to sleep_for() but this might happen during normal operation, so is a good exercise.
It could end up with the time going backwards. If this is undesirable then this may be changed to something like:
SetMockTime(NodeClock::now() + ConsumeIntegralInRange(0, the_argument_to_sleep_for * 2));
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed. <sub>Debug: https://github.com/bitcoin/bitcoin/runs/32589451018</sub>
<details><summary>Hints</summary>
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
</details>
cf83f0c...c97d496: simulate time passage from FuzzedThreadInterrupt::sleep_for() and (hopefully) fix CI
utACK c97d49628a78aac9a65f2bd1ddc733b0b425090b
$ echo "XGQtSi1YAIIkp/8D/yQtJCRYq/9YSv///95cXFz//gABQQAAAABcXCVcXP9cZVxcXFxcXGP//2FkZHL/bWVya2xlYjpISEhISFgjSlgAAQAADABJMCEgIf//5wD+AAsA/wAA/////wAAAQDdAAAAAAAAAAAQAAAJAAAARQD4LgAAABD//gAA/PoAAAAA9v8EAP2n/wP/XGVcXFxcXFxj//8A5wD+AAsA/wD8ZPwjAA==" | base64 --decode > connman.crash
$ FUZZ=connman fuzz connman.crash
==9344==ERROR: MemorySanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f5b532b2c62 bp 0x7ffe2ed01cb0 sp 0x7ffe2ed01468 T9344)
Oops, ConsumeBytes() could return less bytes than requested :facepalm: :face_with_head_bandage:
Fixed:
687a9af2a8...33ffe74302: #31316 (comment)
Thanks!
Please rebase on #31235, fuzzing otherwise leads to:
$ echo "VlZWVlZWVlZWVlZWVhZWVlZWVlZWVlZWVlZWVlZWVlZWVlZWVlZWVlZWVlZWACWcAFwEAAAAZVz9f61hY2v/XHP8BIAI/1z9IiIiIyIiIiIiIiIAIgAACwAAPQ8AAAAAAAAnD29ja3R4AAUAAAAAAAQAAAAAAAAAAAEAAAAAAAAAACAAAAAAOAAAAAAAAAAAAAAAAAAAKm90aAD//wAA//8BAwAAAAAAAAAAAAAAAAAAAND///8AIQAAAAAAACEAAAQAIQAAAAAAXAAAACEAAAARAP8/bH7//wAAAHkAAAAAAAD/ACf/AIN5eVwMAAAAOgHe+6Oj24ejdHgIfmNjY2NjhYWFhY9yYwAAAN6JB/t5eXl5eXl5eXkDAwMAAAAAAAAAAAD+Mf/77zoBAAEAACf/AIN5eVwMAAAAOgEAAUB139773977o6PbeXl5eXl5ef4x///vOgEAAQAAkBEBAQAACH5+fiFcZXYlYWNrAP8hdmX/BXRgY2sHN2hlYWRlcnMAAAAAAAAAUAABAQAkmwAhAEAEAAAAAAAAAND/////////////AAA=" | base64 --decode > connman2.crash
$ FUZZ=connman fuzz connman2.crash
==27516==ERROR: MemorySanitizer: requested allocation size 0x23d7999a4970a3a8 exceeds maximum supported size of 0x10000000000
Additionally:
$ echo "XGRYSlgAgiQkLSQkXSQkJCRYq/9YSv///95cXFwBAAABAAAAAVxcXFz/fGVcXFxcXGNoMWVjRUVFRUX+//9F/0VFdGtw//8HCgAAAAAA5gAAADAhICH//+cA/gALAP8AAP////8AAAEA3QAAAAAAAAAAAAAAAAAA////+gAAAAAQ//4AAPz6AAAAAPb/BAD9p/8D/wcAAAAAADchICH//+cA/gALAP8A/Pz8AxA=" | base64 --decode > connman3.crash
$ FUZZ=connman fuzz connman3.crash
==247==WARNING: MemorySanitizer: use-of-uninitialized-value
[#0](/bitcoin-bitcoin/0/) 0x55adc01e655c in CService::SetSockAddr(sockaddr const*) /workdir/bitcoin/build_fuzz/src/./netaddress.cpp:812:5
[#1](/bitcoin-bitcoin/1/) 0x55adc12ba012 in GetBindAddress(Sock const&) /workdir/bitcoin/build_fuzz/src/./net.cpp:382:19
[#2](/bitcoin-bitcoin/2/) 0x55adc12ddb12 in CConnman::AcceptConnection(CConnman::ListenSocket const&) /workdir/bitcoin/build_fuzz/src/./net.cpp:1722:51
[#3](/bitcoin-bitcoin/3/) 0x55adc12ed808 in CConnman::SocketHandlerListening(std::__1::unordered_map<std::__1::shared_ptr<Sock const>, Sock::Events, Sock::HashSharedPtrSock, Sock::EqualSharedPtrSock, std::__1::allocator<std::__1::pair<std::__1::shared_ptr<Sock const> const, Sock::Events>>> const&) /workdir/bitcoin/build_fuzz/src/./net.cpp:2160:13
[#4](/bitcoin-bitcoin/4/) 0x55adc12e9d48 in CConnman::SocketHandler() /workdir/bitcoin/build_fuzz/src/./net.cpp:2053:5
[#5](/bitcoin-bitcoin/5/) 0x55adbf82d129 in ConnmanTestMsg::SocketHandlerPublic() /workdir/bitcoin/build_fuzz/src/test/fuzz/./test/util/net.h:85:9
[#6](/bitcoin-bitcoin/6/) 0x55adbf82d129 in connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_26::operator()() const /workdir/bitcoin/build_fuzz/src/test/fuzz/./test/fuzz/connman.cpp:211:25
[#7](/bitcoin-bitcoin/7/) 0x55adbf82d129 in unsigned long CallOneOf<connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_4, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_5, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_6, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_7, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_8, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_9, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_10, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_11, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_12, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_13, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_0, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_1, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_14, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_15, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_16, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_17, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_18, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_19, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_20, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_21, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_22, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_23, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_24, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_25, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_26>(FuzzedDataProvider&, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_4, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_5, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_6, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_7, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_8, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_9, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_10, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_11, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_12, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_13, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_0, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_1, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_14, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_15, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_16, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_17, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_18, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_19, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_20, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_21, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_22, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_23, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_24, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_25, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_26) /workdir/bitcoin/build_fuzz/src/test/fuzz/./test/fuzz/util.h:42:27
[#8](/bitcoin-bitcoin/8/) 0x55adbf82d129 in connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>) /workdir/bitcoin/build_fuzz/src/test/fuzz/./test/fuzz/connman.cpp:94:9
[#9](/bitcoin-bitcoin/9/) 0x55adbf5200a5 in decltype(std::declval<void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>)>()(std::declval<std::__1::span<unsigned char const, 18446744073709551615ul>>())) std::__1::__invoke[abi:de190104]<void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>>(void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__type_traits/invoke.h:149:25
[#10](/bitcoin-bitcoin/10/) 0x55adbf5200a5 in void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:de190104]<void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>>(void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__type_traits/invoke.h:224:5
[#11](/bitcoin-bitcoin/11/) 0x55adbf5200a5 in std::__1::__function::__alloc_func<void (*)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::allocator<void (*)(std::__1::span<unsigned char const, 18446744073709551615ul>)>, void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()[abi:de190104](std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__functional/function.h:171:12
[#12](/bitcoin-bitcoin/12/) 0x55adbf5200a5 in std::__1::__function::__func<void (*)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::allocator<void (*)(std::__1::span<unsigned char const, 18446744073709551615ul>)>, void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__functional/function.h:313:10
[#13](/bitcoin-bitcoin/13/) 0x55adc0034ce2 in std::__1::__function::__value_func<void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()[abi:de190104](std::__1::span<unsigned char const, 18446744073709551615ul>&&) const /libcxx_msan/include/c++/v1/__functional/function.h:430:12
[#14](/bitcoin-bitcoin/14/) 0x55adc0034ce2 in std::__1::function<void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::__1::span<unsigned char const, 18446744073709551615ul>) const /libcxx_msan/include/c++/v1/__functional/function.h:989:10
[#15](/bitcoin-bitcoin/15/) 0x55adc0034ce2 in LLVMFuzzerTestOneInput /workdir/bitcoin/build_fuzz/src/test/fuzz/util/./test/fuzz/fuzz.cpp:213:5
[#16](/bitcoin-bitcoin/16/) 0x55adbf409dc6 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
[#17](/bitcoin-bitcoin/17/) 0x55adbf3f3662 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:328:6
[#18](/bitcoin-bitcoin/18/) 0x55adbf3f957f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:863:9
[#19](/bitcoin-bitcoin/19/) 0x55adbf4259a2 in main /llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
[#20](/bitcoin-bitcoin/20/) 0x7f6a6dd60d67 (/lib/x86_64-linux-gnu/libc.so.6+0x29d67) (BuildId: 3bc74dbb72522bb47e0d899e5615140b044a5b40)
[#21](/bitcoin-bitcoin/21/) 0x7f6a6dd60e24 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e24) (BuildId: 3bc74dbb72522bb47e0d899e5615140b044a5b40)
[#22](/bitcoin-bitcoin/22/) 0x55adbf3eb6b0 in _start (/workdir/out/libfuzzer_msan/fuzz+0xf226b0)
Uninitialized value was stored to memory at
[#0](/bitcoin-bitcoin/0/) 0x55adc01e6555 in CService::SetSockAddr(sockaddr const*) /workdir/bitcoin/build_fuzz/src/./netaddress.cpp:812:20
[#1](/bitcoin-bitcoin/1/) 0x55adc12ba012 in GetBindAddress(Sock const&) /workdir/bitcoin/build_fuzz/src/./net.cpp:382:19
[#2](/bitcoin-bitcoin/2/) 0x55adc12ddb12 in CConnman::AcceptConnection(CConnman::ListenSocket const&) /workdir/bitcoin/build_fuzz/src/./net.cpp:1722:51
[#3](/bitcoin-bitcoin/3/) 0x55adc12ed808 in CConnman::SocketHandlerListening(std::__1::unordered_map<std::__1::shared_ptr<Sock const>, Sock::Events, Sock::HashSharedPtrSock, Sock::EqualSharedPtrSock, std::__1::allocator<std::__1::pair<std::__1::shared_ptr<Sock const> const, Sock::Events>>> const&) /workdir/bitcoin/build_fuzz/src/./net.cpp:2160:13
[#4](/bitcoin-bitcoin/4/) 0x55adc12e9d48 in CConnman::SocketHandler() /workdir/bitcoin/build_fuzz/src/./net.cpp:2053:5
[#5](/bitcoin-bitcoin/5/) 0x55adbf82d129 in ConnmanTestMsg::SocketHandlerPublic() /workdir/bitcoin/build_fuzz/src/test/fuzz/./test/util/net.h:85:9
[#6](/bitcoin-bitcoin/6/) 0x55adbf82d129 in connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_26::operator()() const /workdir/bitcoin/build_fuzz/src/test/fuzz/./test/fuzz/connman.cpp:211:25
[#7](/bitcoin-bitcoin/7/) 0x55adbf82d129 in unsigned long CallOneOf<connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_4, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_5, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_6, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_7, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_8, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_9, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_10, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_11, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_12, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_13, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_0, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_1, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_14, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_15, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_16, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_17, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_18, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_19, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_20, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_21, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_22, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_23, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_24, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_25, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_26>(FuzzedDataProvider&, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_4, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_5, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_6, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_7, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_8, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_9, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_10, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_11, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_12, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_13, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_0, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_1, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_14, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_15, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_16, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_17, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_18, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_19, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_20, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_21, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_22, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_23, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_24, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_25, connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>)::$_26) /workdir/bitcoin/build_fuzz/src/test/fuzz/./test/fuzz/util.h:42:27
[#8](/bitcoin-bitcoin/8/) 0x55adbf82d129 in connman_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>) /workdir/bitcoin/build_fuzz/src/test/fuzz/./test/fuzz/connman.cpp:94:9
[#9](/bitcoin-bitcoin/9/) 0x55adbf5200a5 in decltype(std::declval<void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>)>()(std::declval<std::__1::span<unsigned char const, 18446744073709551615ul>>())) std::__1::__invoke[abi:de190104]<void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>>(void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__type_traits/invoke.h:149:25
[#10](/bitcoin-bitcoin/10/) 0x55adbf5200a5 in void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:de190104]<void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>>(void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__type_traits/invoke.h:224:5
[#11](/bitcoin-bitcoin/11/) 0x55adbf5200a5 in std::__1::__function::__alloc_func<void (*)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::allocator<void (*)(std::__1::span<unsigned char const, 18446744073709551615ul>)>, void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()[abi:de190104](std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__functional/function.h:171:12
[#12](/bitcoin-bitcoin/12/) 0x55adbf5200a5 in std::__1::__function::__func<void (*)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::allocator<void (*)(std::__1::span<unsigned char const, 18446744073709551615ul>)>, void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__functional/function.h:313:10
[#13](/bitcoin-bitcoin/13/) 0x55adc0034ce2 in std::__1::__function::__value_func<void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()[abi:de190104](std::__1::span<unsigned char const, 18446744073709551615ul>&&) const /libcxx_msan/include/c++/v1/__functional/function.h:430:12
[#14](/bitcoin-bitcoin/14/) 0x55adc0034ce2 in std::__1::function<void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::__1::span<unsigned char const, 18446744073709551615ul>) const /libcxx_msan/include/c++/v1/__functional/function.h:989:10
[#15](/bitcoin-bitcoin/15/) 0x55adc0034ce2 in LLVMFuzzerTestOneInput /workdir/bitcoin/build_fuzz/src/test/fuzz/util/./test/fuzz/fuzz.cpp:213:5
[#16](/bitcoin-bitcoin/16/) 0x55adbf409dc6 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
[#17](/bitcoin-bitcoin/17/) 0x55adbf3f3662 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:328:6
[#18](/bitcoin-bitcoin/18/) 0x55adbf3f957f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:863:9
[#19](/bitcoin-bitcoin/19/) 0x55adbf4259a2 in main /llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
[#20](/bitcoin-bitcoin/20/) 0x7f6a6dd60d67 (/lib/x86_64-linux-gnu/libc.so.6+0x29d67) (BuildId: 3bc74dbb72522bb47e0d899e5615140b044a5b40)
Uninitialized value was created by an allocation of 'sockaddr_bind' in the stack frame
[#0](/bitcoin-bitcoin/0/) 0x55adc12b9e9e in GetBindAddress(Sock const&) /workdir/bitcoin/build_fuzz/src/./net.cpp:379:5
SUMMARY: MemorySanitizer: use-of-uninitialized-value /workdir/bitcoin/build_fuzz/src/./netaddress.cpp:812:5 in CService::SetSockAddr(sockaddr const*)
Probably caused by FuzzedSock::GetSockName using ConsumeData on an empty data provider (i.e. sockaddr* name won't be initialized):
33ffe74302...72ff6d2b50:
Please rebase on #31235
Done. Rebasing fixed connman2.crash for me.
I can't reproduce connman3.crash (!?) but anyway I can see the problem with FuzzedSock::GetSockName() initializing too little of the output, so I changed it to fully set the entire output, always.
I can't reproduce connman3.crash
You'll need memory sanitizer instrumentation for that one, which unfortunately requires all dependencies (including libc++) to be instrumented as well, so it's a pain to setup. I'll re-test the fix for you!
Shouldn't valgrind be able to detect UB memory issues, unless they were optimized out?
72ff6d2b50...696b6671da: rebase due to conflicts
696b6671da...7e42c92d2b: rebase due to conflicts
7e42c92d2b...582d9e3dbd: rebase due to conflicts
I believe all issues here have been addressed and this is ready for review. Has a stale ACK from @brunoerg and @jonatack and Concept ACK from @dergoegge.
224 | @@ -225,6 +225,18 @@ inline CService ConsumeService(FuzzedDataProvider& fuzzed_data_provider) noexcep 225 | return {ConsumeNetAddr(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<uint16_t>()}; 226 | } 227 | 228 | +inline std::vector<CService> ConsumeServiceVector(FuzzedDataProvider& fuzzed_data_provider,
d790b83996ae0d0bc4fe9723606c8c923cec1bcc: nit: It seems the ConsumeServiceVector is always called with the max_vector_size being 5? Maybe it could be the default value since it's only used here.
Done, thanks!
reACK 582d9e3dbdf5b64272b65844d679942c5aca643f
Just generated this coverage report with some hours of running: https://brunoerg.xyz/bitcoin-core-coverage/28584/coverage_report/
75 | + fuzzed_data_provider.ConsumeBool(), 76 | + ConsumeThreadInterrupt(fuzzed_data_provider)}; 77 | 78 | const uint64_t max_outbound_limit{fuzzed_data_provider.ConsumeIntegral<uint64_t>()}; 79 | CConnman::Options options; 80 | + options.m_msgproc = g_setup->m_node.peerman.get();
It'd be better to create a fake NetEventsInterface for this test, as we are testing connman not peerman. Peerman is also a global here which might have unwanted side effects.
I haven't tried running the most recent version of this test but it looks like it'll run out of memory eventually as the Peers allocated in peerman are never deleted?
Added a fake/fuzzed NetEventsInterface that is created and destroyed per every run, like the test connman itself. That addresses the "out of memory eventually" concern as well. Thanks!
`FuzzedSock::Accept()` properly returns a new socket, but it forgot to
set the output argument `addr`, like `accept(2)` is expected to.
This could lead to reading uninitialized data during testing when we
read it, e.g. from `CService::SetSockAddr()` which reads the `sa_family`
member.
Set `addr` to a fuzzed IPv4 or IPv6 address.
Now that all network calls done by `CConnman::OpenNetworkConnection()`
are done via `Sock` they can be redirected (mocked) to `FuzzedSocket`
for testing.
* Make the methods of `CThreadInterrupt` virtual and store a pointer to
it in `CConnman`, thus making it possible to override with a mocked
instance.
* Initialize `CConnman::m_interrupt_net` from the constructor, making it
possible for callers to supply mocked version.
* Introduce `FuzzedThreadInterrupt` and `ConsumeThreadInterrupt()` and
use them in `src/test/fuzz/connman.cpp` and `src/test/fuzz/i2p.cpp`.
This improves the CPU utilization of the `connman` fuzz test.
As a nice side effect, the `std::shared_ptr` used for
`CConnman::m_interrupt_net` resolves the possible lifetime issues with
it (see the removed comment for that variable).
582d9e3dbd...0802398e74: rebase and address suggestions
Review re-ACK 0802398e749c5e16fa7085cd87c91a31bbe043bd
44 | + 45 | + /// Interrupt any sleeps. After this `interrupted()` will return `true`. 46 | + virtual void operator()() EXCLUSIVE_LOCKS_REQUIRED(!mut); 47 | + 48 | + /// Reset to an non-interrupted state. 49 | + virtual void reset();
nit: it might make sense to name this differently, just to avoid someone confusing this function with a smart ptr's reset().
Good point! I do not know why I chose lowercase since it also violates the coding style. Maybe it was like that before and I left it as it is even though I touched those lines and the callers.
Append the following as a new commit or amend it into the last one fuzz: make it possible to mock (fuzz) CThreadInterrupt?
<details> <summary>[patch] Uppercase methods of CThreadInterrupt</summary>
commit 486db14b9ce13a90b69f69dafb59d3d8932498f7 (HEAD -> fuzz_connman)
Parent: 0802398e749c5e16fa7085cd87c91a31bbe043bd
Author: Vasil Dimov <vd@FreeBSD.org>
AuthorDate: Tue Sep 2 13:40:35 2025 +0200
Commit: Vasil Dimov <vd@FreeBSD.org>
CommitDate: Tue Sep 2 13:40:35 2025 +0200
gpg: Signature made Tue Sep 2 13:42:45 2025 CEST
gpg: using RSA key E64D8D45614DB07545D9CCC154DF06F64B55CBBF
gpg: Good signature from "Vasil Dimov <vd@myforest.net>" [ultimate]
gpg: aka "Vasil Dimov <vd@FreeBSD.org>" [ultimate]
gpg: aka "Vasil Dimov <vasild@gmail.com>" [ultimate]
util: Uppercase methods of CThreadInterrupt
Rename
`CThreadInterrupt::interrupted()` -> `CThreadInterrupt::Interrupted()`
`CThreadInterrupt::reset()` -> `CThreadInterrupt::Reset()`
`CThreadInterrupt::sleep_for()` -> `CThreadInterrupt::SleepFor()`
for consistency with the coding style and to avoid confusing the
`reset()` method of `CThreadInterrupt` with the `reset()` methods of
smart pointers.
diff --git a/src/i2p.cpp b/src/i2p.cpp
index 80f3bde4cf..2ebd0b8c45 100644
--- a/src/i2p.cpp
+++ b/src/i2p.cpp
@@ -159,13 +159,13 @@ bool Session::Accept(Connection& conn)
{
AssertLockNotHeld(m_mutex);
std::string errmsg;
bool disconnect{false};
- while (!m_interrupt->interrupted()) {
+ while (!m_interrupt->Interrupted()) {
Sock::Event occurred;
if (!conn.sock->Wait(MAX_WAIT_FOR_IO, Sock::RECV, &occurred)) {
errmsg = "wait on socket failed";
break;
}
@@ -202,13 +202,13 @@ bool Session::Accept(Connection& conn)
conn.peer = CService(peer_addr, I2P_SAM31_PORT);
return true;
}
- if (m_interrupt->interrupted()) {
+ if (m_interrupt->Interrupted()) {
LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Accept was interrupted\n");
} else {
LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error accepting%s: %s\n", disconnect ? " (will close the session)" : "", errmsg);
}
if (disconnect) {
LOCK(m_mutex);
diff --git a/src/index/base.cpp b/src/index/base.cpp
index 5767116ce5..cd36967ac6 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -83,13 +83,13 @@ BaseIndex::~BaseIndex()
bool BaseIndex::Init()
{
AssertLockNotHeld(cs_main);
// May need reset if index is being restarted.
- m_interrupt.reset();
+ m_interrupt.Reset();
// m_chainstate member gives indexing code access to node internals. It is
// removed in followup [#24230](/bitcoin-bitcoin/24230/)
m_chainstate = WITH_LOCK(::cs_main,
return &m_chain->context()->chainman->GetChainstateForIndexing());
// Register to validation interface before setting the 'm_synced' flag, so that
diff --git a/src/mapport.cpp b/src/mapport.cpp
index 83105f51fd..4d2f1f1957 100644
--- a/src/mapport.cpp
+++ b/src/mapport.cpp
@@ -110,23 +110,23 @@ static void ProcessPCP()
}
// RFC6887 11.2.1 recommends that clients send their first renewal packet at a time chosen with uniform random
// distribution in the range 1/2 to 5/8 of expiration time.
std::chrono::seconds sleep_time_min(actual_lifetime / 2);
std::chrono::seconds sleep_time_max(actual_lifetime * 5 / 8);
sleep_time = sleep_time_min + FastRandomContext().randrange<std::chrono::milliseconds>(sleep_time_max - sleep_time_min);
- } while (ret && g_mapport_interrupt.sleep_for(sleep_time));
+ } while (ret && g_mapport_interrupt.SleepFor(sleep_time));
// We don't delete the mappings when the thread is interrupted because this would add additional complexity, so
// we rather just choose a fairly short expiry time.
}
static void ThreadMapPort()
{
do {
ProcessPCP();
- } while (g_mapport_interrupt.sleep_for(PORT_MAPPING_RETRY_PERIOD));
+ } while (g_mapport_interrupt.SleepFor(PORT_MAPPING_RETRY_PERIOD));
}
void StartThreadMapPort()
{
if (!g_mapport_thread.joinable()) {
assert(!g_mapport_interrupt);
@@ -152,9 +152,9 @@ void InterruptMapPort()
}
void StopMapPort()
{
if (g_mapport_thread.joinable()) {
g_mapport_thread.join();
- g_mapport_interrupt.reset();
+ g_mapport_interrupt.Reset();
}
}
diff --git a/src/net.cpp b/src/net.cpp
index 85c25543d3..ee34b09ebd 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -2091,13 +2091,13 @@ void CConnman::SocketHandler()
// Check for the readiness of the already connected sockets and the
// listening sockets in one call ("readiness" as in poll(2) or
// select(2)). If none are ready, wait for a short while and return
// empty sets.
events_per_sock = GenerateWaitSockets(snap.Nodes());
if (events_per_sock.empty() || !events_per_sock.begin()->first->WaitMany(timeout, events_per_sock)) {
- m_interrupt_net->sleep_for(timeout);
+ m_interrupt_net->SleepFor(timeout);
}
// Service (send/receive) each of the already connected nodes.
SocketHandlerConnected(snap.Nodes(), events_per_sock);
}
@@ -2108,13 +2108,13 @@ void CConnman::SocketHandler()
void CConnman::SocketHandlerConnected(const std::vector<CNode*>& nodes,
const Sock::EventsPerSock& events_per_sock)
{
AssertLockNotHeld(m_total_bytes_sent_mutex);
for (CNode* pnode : nodes) {
- if (m_interrupt_net->interrupted()) {
+ if (m_interrupt_net->Interrupted()) {
return;
}
//
// Receive
//
@@ -2205,13 +2205,13 @@ void CConnman::SocketHandlerConnected(const std::vector<CNode*>& nodes,
}
}
void CConnman::SocketHandlerListening(const Sock::EventsPerSock& events_per_sock)
{
for (const ListenSocket& listen_socket : vhListenSocket) {
- if (m_interrupt_net->interrupted()) {
+ if (m_interrupt_net->Interrupted()) {
return;
}
const auto it = events_per_sock.find(listen_socket.sock);
if (it != events_per_sock.end() && it->second.occurred & Sock::RECV) {
AcceptConnection(listen_socket);
}
@@ -2219,13 +2219,13 @@ void CConnman::SocketHandlerListening(const Sock::EventsPerSock& events_per_sock
}
void CConnman::ThreadSocketHandler()
{
AssertLockNotHeld(m_total_bytes_sent_mutex);
- while (!m_interrupt_net->interrupted()) {
+ while (!m_interrupt_net->Interrupted()) {
DisconnectNodes();
NotifyNumConnectionsChanged();
SocketHandler();
}
}
@@ -2243,14 +2243,14 @@ void CConnman::ThreadDNSAddressSeed()
int outbound_connection_count = 0;
if (!gArgs.GetArgs("-seednode").empty()) {
auto start = NodeClock::now();
constexpr std::chrono::seconds SEEDNODE_TIMEOUT = 30s;
LogPrintf("-seednode enabled. Trying the provided seeds for %d seconds before defaulting to the dnsseeds.\n", SEEDNODE_TIMEOUT.count());
- while (!m_interrupt_net->interrupted()) {
- if (!m_interrupt_net->sleep_for(500ms)) {
+ while (!m_interrupt_net->Interrupted()) {
+ if (!m_interrupt_net->SleepFor(500ms)) {
return;
}
// Abort if we have spent enough time without reaching our target.
// Giving seed nodes 30 seconds so this does not become a race against fixedseeds (which triggers after 1 min)
if (NodeClock::now() > start + SEEDNODE_TIMEOUT) {
@@ -2307,13 +2307,13 @@ void CConnman::ThreadDNSAddressSeed()
std::chrono::seconds to_wait = seeds_wait_time;
while (to_wait.count() > 0) {
// if sleeping for the MANY_PEERS interval, wake up
// early to see if we have enough peers and can stop
// this thread entirely freeing up its resources
std::chrono::seconds w = std::min(DNSSEEDS_DELAY_FEW_PEERS, to_wait);
- if (!m_interrupt_net->sleep_for(w)) return;
+ if (!m_interrupt_net->SleepFor(w)) return;
to_wait -= w;
if (GetFullOutboundConnCount() >= SEED_OUTBOUND_CONNECTION_THRESHOLD) {
if (found > 0) {
LogPrintf("%d addresses found from DNS seeds\n", found);
LogPrintf("P2P peers available. Finished DNS seeding.\n");
@@ -2323,19 +2323,19 @@ void CConnman::ThreadDNSAddressSeed()
return;
}
}
}
}
- if (m_interrupt_net->interrupted()) return;
+ if (m_interrupt_net->Interrupted()) return;
// hold off on querying seeds if P2P network deactivated
if (!fNetworkActive) {
LogPrintf("Waiting for network to be reactivated before querying DNS seeds.\n");
do {
- if (!m_interrupt_net->sleep_for(1s)) return;
+ if (!m_interrupt_net->SleepFor(1s)) return;
} while (!fNetworkActive);
}
LogPrintf("Loading addresses from DNS seed %s\n", seed);
// If -proxy is in use, we make an ADDR_FETCH connection to the DNS resolved peer address
// for the base dns seed domain in chainparams
@@ -2524,18 +2524,18 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, std
for (const std::string& strAddr : connect)
{
CAddress addr(CService(), NODE_NONE);
OpenNetworkConnection(addr, false, {}, strAddr.c_str(), ConnectionType::MANUAL, /*use_v2transport=*/use_v2transport);
for (int i = 0; i < 10 && i < nLoop; i++)
{
- if (!m_interrupt_net->sleep_for(500ms)) {
+ if (!m_interrupt_net->SleepFor(500ms)) {
return;
}
}
}
- if (!m_interrupt_net->sleep_for(500ms)) {
+ if (!m_interrupt_net->SleepFor(500ms)) {
return;
}
PerformReconnections();
}
}
@@ -2555,13 +2555,13 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, std
constexpr std::chrono::seconds ADD_NEXT_SEEDNODE = 10s;
if (!add_fixed_seeds) {
LogPrintf("Fixed seeds are disabled\n");
}
- while (!m_interrupt_net->interrupted()) {
+ while (!m_interrupt_net->Interrupted()) {
if (add_addr_fetch) {
add_addr_fetch = false;
const auto& seed{SpanPopBack(seed_nodes)};
AddAddrFetch(seed);
if (addrman.Size() == 0) {
@@ -2570,20 +2570,20 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, std
LogInfo("Couldn't connect to peers from addrman after %d seconds. Adding seednode (%s) to addrfetch\n", ADD_NEXT_SEEDNODE.count(), seed);
}
}
ProcessAddrFetch();
- if (!m_interrupt_net->sleep_for(500ms)) {
+ if (!m_interrupt_net->SleepFor(500ms)) {
return;
}
PerformReconnections();
CountingSemaphoreGrant<> grant(*semOutbound);
- if (m_interrupt_net->interrupted()) {
+ if (m_interrupt_net->Interrupted()) {
return;
}
const std::unordered_set<Network> fixed_seed_networks{GetReachableEmptyNetworks()};
if (add_fixed_seeds && !fixed_seed_networks.empty()) {
// When the node starts with an empty peers.dat, there are a few other sources of peers before
@@ -2753,13 +2753,13 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, std
addrman.ResolveCollisions();
const auto current_time{NodeClock::now()};
int nTries = 0;
const auto reachable_nets{g_reachable_nets.All()};
- while (!m_interrupt_net->interrupted()) {
+ while (!m_interrupt_net->Interrupted()) {
if (anchor && !m_anchors.empty()) {
const CAddress addr = m_anchors.back();
m_anchors.pop_back();
if (!addr.IsValid() || IsLocal(addr) || !g_reachable_nets.Contains(addr) ||
!m_msgproc->HasAllDesirableServiceFlags(addr.nServices) ||
outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr))) continue;
@@ -2855,13 +2855,13 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, std
break;
}
if (addrConnect.IsValid()) {
if (fFeeler) {
// Add small amount of random noise before connection to avoid synchronization.
- if (!m_interrupt_net->sleep_for(rng.rand_uniform_duration<CThreadInterrupt::Clock>(FEELER_SLEEP_WINDOW))) {
+ if (!m_interrupt_net->SleepFor(rng.rand_uniform_duration<CThreadInterrupt::Clock>(FEELER_SLEEP_WINDOW))) {
return;
}
LogDebug(BCLog::NET, "Making feeler connection to %s\n", addrConnect.ToStringAddrPort());
}
if (preferred_net != std::nullopt) LogDebug(BCLog::NET, "Making network specific connection to %s on %s.\n", addrConnect.ToStringAddrPort(), GetNetworkName(preferred_net.value()));
@@ -2966,19 +2966,19 @@ void CConnman::ThreadOpenAddedConnections()
// the addednodeinfo state might change.
break;
}
tried = true;
CAddress addr(CService(), NODE_NONE);
OpenNetworkConnection(addr, false, std::move(grant), info.m_params.m_added_node.c_str(), ConnectionType::MANUAL, info.m_params.m_use_v2transport);
- if (!m_interrupt_net->sleep_for(500ms)) return;
+ if (!m_interrupt_net->SleepFor(500ms)) return;
grant = CountingSemaphoreGrant<>(*semAddnode, /*fTry=*/true);
}
// See if any reconnections are desired.
PerformReconnections();
// Retry every 60 seconds if a connection was attempted, otherwise two seconds
- if (!m_interrupt_net->sleep_for(tried ? 60s : 2s)) {
+ if (!m_interrupt_net->SleepFor(tried ? 60s : 2s)) {
return;
}
}
}
// if successful, this moves the passed grant to the constructed node
@@ -2987,13 +2987,13 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
AssertLockNotHeld(m_unused_i2p_sessions_mutex);
assert(conn_type != ConnectionType::INBOUND);
//
// Initiate outbound network connection
//
- if (m_interrupt_net->interrupted()) {
+ if (m_interrupt_net->Interrupted()) {
return;
}
if (!fNetworkActive) {
return;
}
if (!pszDest) {
@@ -3075,19 +3075,19 @@ void CConnman::ThreadI2PAcceptIncoming()
auto err_wait = err_wait_begin;
bool advertising_listen_addr = false;
i2p::Connection conn;
auto SleepOnFailure = [&]() {
- m_interrupt_net->sleep_for(err_wait);
+ m_interrupt_net->SleepFor(err_wait);
if (err_wait < err_wait_cap) {
err_wait += 1s;
}
};
- while (!m_interrupt_net->interrupted()) {
+ while (!m_interrupt_net->Interrupted()) {
if (!m_i2p_sam_session->Listen(conn)) {
if (advertising_listen_addr && conn.me.IsValid()) {
RemoveLocal(conn.me);
advertising_listen_addr = false;
}
@@ -3347,13 +3347,13 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
}
//
// Start threads
//
assert(m_msgproc);
- m_interrupt_net->reset();
+ m_interrupt_net->Reset();
flagInterruptMsgProc = false;
{
LOCK(mutexMsgProc);
fMsgProcWake = false;
}
diff --git a/src/test/fuzz/util/threadinterrupt.cpp b/src/test/fuzz/util/threadinterrupt.cpp
index 5dd87e0588..5e23402447 100644
--- a/src/test/fuzz/util/threadinterrupt.cpp
+++ b/src/test/fuzz/util/threadinterrupt.cpp
@@ -7,16 +7,16 @@
FuzzedThreadInterrupt::FuzzedThreadInterrupt(FuzzedDataProvider& fuzzed_data_provider)
: m_fuzzed_data_provider{fuzzed_data_provider}
{
}
-bool FuzzedThreadInterrupt::interrupted() const
+bool FuzzedThreadInterrupt::Interrupted() const
{
return m_fuzzed_data_provider.ConsumeBool();
}
-bool FuzzedThreadInterrupt::sleep_for(Clock::duration)
+bool FuzzedThreadInterrupt::SleepFor(Clock::duration)
{
SetMockTime(ConsumeTime(m_fuzzed_data_provider)); // Time could go backwards.
return m_fuzzed_data_provider.ConsumeBool();
}
diff --git a/src/test/fuzz/util/threadinterrupt.h b/src/test/fuzz/util/threadinterrupt.h
index d56aefd919..71ce2ddd3f 100644
--- a/src/test/fuzz/util/threadinterrupt.h
+++ b/src/test/fuzz/util/threadinterrupt.h
@@ -15,14 +15,14 @@
*/
class FuzzedThreadInterrupt : public CThreadInterrupt
{
public:
explicit FuzzedThreadInterrupt(FuzzedDataProvider& fuzzed_data_provider);
- virtual bool interrupted() const override;
- virtual bool sleep_for(Clock::duration) override;
+ virtual bool Interrupted() const override;
+ virtual bool SleepFor(Clock::duration) override;
private:
FuzzedDataProvider& m_fuzzed_data_provider;
};
[[nodiscard]] inline std::shared_ptr<CThreadInterrupt> ConsumeThreadInterrupt(FuzzedDataProvider& fuzzed_data_provider)
diff --git a/src/util/threadinterrupt.cpp b/src/util/threadinterrupt.cpp
index aaa9e831a9..8402181cf0 100644
--- a/src/util/threadinterrupt.cpp
+++ b/src/util/threadinterrupt.cpp
@@ -6,23 +6,23 @@
#include <util/threadinterrupt.h>
#include <sync.h>
CThreadInterrupt::CThreadInterrupt() : flag(false) {}
-bool CThreadInterrupt::interrupted() const
+bool CThreadInterrupt::Interrupted() const
{
return flag.load(std::memory_order_acquire);
}
CThreadInterrupt::operator bool() const
{
- return interrupted();
+ return Interrupted();
}
-void CThreadInterrupt::reset()
+void CThreadInterrupt::Reset()
{
flag.store(false, std::memory_order_release);
}
void CThreadInterrupt::operator()()
{
@@ -30,11 +30,11 @@ void CThreadInterrupt::operator()()
LOCK(mut);
flag.store(true, std::memory_order_release);
}
cond.notify_all();
}
-bool CThreadInterrupt::sleep_for(Clock::duration rel_time)
+bool CThreadInterrupt::SleepFor(Clock::duration rel_time)
{
WAIT_LOCK(mut, lock);
return !cond.wait_for(lock, rel_time, [this]() { return flag.load(std::memory_order_acquire); });
}
diff --git a/src/util/threadinterrupt.h b/src/util/threadinterrupt.h
index 8b393c26df..e11ff1881b 100644
--- a/src/util/threadinterrupt.h
+++ b/src/util/threadinterrupt.h
@@ -30,27 +30,27 @@ public:
CThreadInterrupt();
virtual ~CThreadInterrupt() = default;
/// Return true if `operator()()` has been called.
- virtual bool interrupted() const;
+ virtual bool Interrupted() const;
/// An alias for `interrupted()`.
virtual explicit operator bool() const;
/// Interrupt any sleeps. After this `interrupted()` will return `true`.
virtual void operator()() EXCLUSIVE_LOCKS_REQUIRED(!mut);
/// Reset to an non-interrupted state.
- virtual void reset();
+ virtual void Reset();
/// Sleep for the given duration.
/// [@retval](/bitcoin-bitcoin/contributor/retval/) true The time passed.
/// [@retval](/bitcoin-bitcoin/contributor/retval/) false The sleep was interrupted.
- virtual bool sleep_for(Clock::duration rel_time) EXCLUSIVE_LOCKS_REQUIRED(!mut);
+ virtual bool SleepFor(Clock::duration rel_time) EXCLUSIVE_LOCKS_REQUIRED(!mut);
private:
std::condition_variable cond;
Mutex mut;
std::atomic<bool> flag;
};
</details>
Code review ACK 0802398e749c5e16fa7085cd87c91a31bbe043bd
ACK 0802398e749c5e16fa7085cd87c91a31bbe043bd
208 | + options.onion_binds.empty(); 209 | + 210 | + connman.InitBindsPublic(options); 211 | + }, 212 | + [&] { 213 | + connman.SocketHandlerPublic();
# echo 'XGFkZAAAAGRkZWXuXP/fcGcqb2hlcirYfg9D/uXc5eXcRZJ55eXl5eXl5eXlIiL19QAFABD3XERc
AVxhYQcAAADl5f//5eVhYWHl5eX//+Xl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl
5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eXl5eX/Km8xMTQyMjgxMUMKYWFhYWFhYQAAAAAA
YWFhYWFhYWFhYWFhYWFhe2FhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFh8mWkovx0AAAA
AAAAAGFhYWFhYWFhYWFhgKoL//v/Kv/////l5eXl5f//ZGRy5eX//2Ry5eX///9kZHLl5f//ZHLl
5f//5eXl5eXl5eXl5Wfl//9kZHLl5f//ZHLl5f///2RkcuXl//9kcuXl//8=' | base64 --decode > ./crash_cm_1cfcffc33a
# UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=connman ./bld/bin/fuzz -runs=1 ./crash_cm_1cfcffc33a
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2899209193
INFO: Loaded 1 modules (597578 inline 8-bit counters): 597578 [0x62ee33b00588, 0x62ee33b923d2),
INFO: Loaded 1 PC tables (597578 PCs): 597578 [0x62ee33b923d8,0x62ee344b0878),
./bld/bin/fuzz: Running 1 inputs 1 time(s) each.
Running: ./crash_cm_1cfcffc33a
./src/test/fuzz/util/net.cpp:337:43: runtime error: null pointer passed as argument 2, which is declared to never be null
Just for reference: issue: #33643 fix: https://github.com/bitcoin/bitcoin/pull/33644