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).
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
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.
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.
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.
@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.
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);
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.
0diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h
1index a97017555d..78e61b51d9 100644
2--- a/src/test/fuzz/util/net.h
3+++ b/src/test/fuzz/util/net.h
4@@ -102,8 +102,7 @@ inline std::vector<CService> ConsumeServiceVector(FuzzedDataProvider& fuzzed_dat
5 const size_t size = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, max_vector_size);
6 ret.reserve(size);
7 for (size_t i = 0; i < size; ++i) {
8- ret.emplace_back(ConsumeNetAddr(fuzzed_data_provider),
9- fuzzed_data_provider.ConsumeIntegral<uint16_t>());
10+ ret.emplace_back(ConsumeService(fuzzed_data_provider));
11 }
12 return ret;
13 }
cd5bbb12e0...5e3c80da14: address suggestions
🚧 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.
5e3c80da14...6d9083b249: rebase due to silent merge conflict
🚧 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.
6d9083b249...45f4dbe484: rebase due to conflicts
Now that all network calls done by `CConnman::OpenNetworkConnection()`
are done via `Sock` they can be redirected (mocked) to `FuzzedSocket`
for testing.
45f4dbe484...655a2cf666: the previous push resolved the merge conflict in a too late commit, causing the “test each commit” CI job to fail