fuzz: connman: strengthen assertions and extend coverage #35220

pull brunoerg wants to merge 5 commits into bitcoin:master from brunoerg:2026-04-fuzz-connman-asserts changing 1 files +54 −13
  1. brunoerg commented at 8:39 AM on May 6, 2026: contributor

    This PR improves the connman fuzz target by replacing some "(void)" calls with actual invariant checks, adding coverage for previously uncovered methods, and exercising more initialization states.

    • Set m_local_services, m_use_addrman_outgoing, and m_max_automatic_connections via fuzzed values before Init() to explore more startup configurations.

    • Add network activity and outbound-bytes invariants.

    • Add AddNode/RemoveAddedNode invariants: e.g. a successful AddNode increases GetAddedNodeInfo() by one; adding the same node again must fail; a subsequent RemoveAddedNode must succeed and restore the original count.

    • Add coverage for AddLocalServices/RemoveLocalServices.

  2. fuzz: connman: add network activity invariants dd2eca0f34
  3. fuzz: connman: set m_local_services/m_use_addrman_outgoing/m_max_automatic_connections de573fa784
  4. fuzz: connman: add AddNode/RemoveAddedNode invariants 28648e31c6
  5. fuzz: connman: add outbound-bytes invariants a4287bd6fc
  6. fuzz: connman: cover AddLocalServices/RemoveLocalServices dfd3744b7b
  7. DrahtBot added the label Fuzzing on May 6, 2026
  8. DrahtBot commented at 8:39 AM on May 6, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35220.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK frankomosh

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #34934 (fuzz: exercise ForNode/ForEachNode callbacks in connman fuzz harness by frankomosh)

    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-->

  9. frankomosh commented at 2:38 PM on May 11, 2026: contributor

    Concept ACK. I think it is a good thing to have these assertions. Will review each one of them separately.

  10. in src/test/fuzz/connman.cpp:271 in dfd3744b7b
     271 | -    (void)connman.GetMaxOutboundTimeframe();
     272 | -    (void)connman.GetMaxOutboundTimeLeftInCycle();
     273 | -    (void)connman.GetNetworkActive();
     274 | +    const auto max_outbound_timeframe{connman.GetMaxOutboundTimeframe()};
     275 | +    const auto time_left_in_cycle{connman.GetMaxOutboundTimeLeftInCycle()};
     276 | +    assert(time_left_in_cycle <= max_outbound_timeframe);
    


    marcofleon commented at 5:39 PM on May 13, 2026:

    If we're using FuzzedThreadInterrupt::sleep_for for fuzzing here, then time could go backwards so not sure if this assertion ends up making much sense.

    <details> <summary>stack trace</summary>

    New crash id='c1a9eafd29f404091ffb4ea9ff64f01cc82e49b9' (project='bitcoin' harness='connman')
    Base64: XJB5XFxcXABbXFxcJ1xcKgB5//////9381l5AHkkeP//d/NZeQBbXNBcXCdcXCr9/f39/f39/f39/f39/f39/f39eVwxMTFCMTExMYoAef//d/Jc+////yoAef//////d/NZeQBbXFxcMV0xojExKTExAwAAAAAAAAA//SQAeYKGXFxcAQAACFx5Knl5eVt5igB5//938ll5AFtcXFxcJ1xcKnl5eVs/eSQA/YKGXG8AAAAAAAD///8IAAAAZ2V0YmxvY2t0AAAAAAAAAJCQkJCQkJCQXCg=
    ===== stack trace ===== 
    INFO: Running with entropic power schedule (0xFF, 100).
    INFO: Seed: 551831089
    INFO: Loaded 1 modules   (523390 inline 8-bit counters): 523390 [0x559145f62880, 0x559145fe24fe), 
    INFO: Loaded 1 PC tables (523390 PCs): 523390 [0x559145fe2500,0x5591467dece0), 
    /workdir/out/libfuzzer_ubsan/fuzz: Running 1 inputs 1 time(s) each.
    Running: /workdir/workspace/solutions/crash-f0a3fbcd8af1f458dd289f8154f89e3d8e4279c0
    fuzz: test/fuzz/connman.cpp:271: void connman_fuzz_target(FuzzBufferType): Assertion `time_left_in_cycle <= max_outbound_timeframe' failed.
    ==2604== ERROR: libFuzzer: deadly signal
        [#0](/github-metadata-backup-bitcoin-bitcoin/0/) 0x55914445aaf4 in __sanitizer_print_stack_trace /llvm-project/compiler-rt/lib/ubsan/ubsan_diag_standalone.cpp:31:3
        [#1](/github-metadata-backup-bitcoin-bitcoin/1/) 0x5591443cc178 in fuzzer::PrintStackTrace() /llvm-project/compiler-rt/lib/fuzzer/FuzzerUtil.cpp:210:5
        [#2](/github-metadata-backup-bitcoin-bitcoin/2/) 0x5591443aeb93 in fuzzer::Fuzzer::CrashCallback() /llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:231:3
        [#3](/github-metadata-backup-bitcoin-bitcoin/3/) 0x7ff09eb77def  (/lib/x86_64-linux-gnu/libc.so.6+0x3fdef) (BuildId: 58749c528985eab03e6700ebc1469fa50aa41219)
        [#4](/github-metadata-backup-bitcoin-bitcoin/4/) 0x7ff09ebcc95b  (/lib/x86_64-linux-gnu/libc.so.6+0x9495b) (BuildId: 58749c528985eab03e6700ebc1469fa50aa41219)
        [#5](/github-metadata-backup-bitcoin-bitcoin/5/) 0x7ff09eb77cc1 in raise (/lib/x86_64-linux-gnu/libc.so.6+0x3fcc1) (BuildId: 58749c528985eab03e6700ebc1469fa50aa41219)
        [#6](/github-metadata-backup-bitcoin-bitcoin/6/) 0x7ff09eb604ab in abort (/lib/x86_64-linux-gnu/libc.so.6+0x284ab) (BuildId: 58749c528985eab03e6700ebc1469fa50aa41219)
        [#7](/github-metadata-backup-bitcoin-bitcoin/7/) 0x7ff09eb6041f  (/lib/x86_64-linux-gnu/libc.so.6+0x2841f) (BuildId: 58749c528985eab03e6700ebc1469fa50aa41219)
        [#8](/github-metadata-backup-bitcoin-bitcoin/8/) 0x5591445aa080 in connman_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) /workdir/bitcoin/src/test/fuzz/connman.cpp:271:5
        [#9](/github-metadata-backup-bitcoin-bitcoin/9/) 0x559144915aed in std::function<void (std::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::span<unsigned char const, 18446744073709551615ul>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/std_function.h:591:9
        [#10](/github-metadata-backup-bitcoin-bitcoin/10/) 0x559144915aed in test_one_input(std::span<unsigned char const, 18446744073709551615ul>) /workdir/bitcoin/src/test/fuzz/fuzz.cpp:86:5
        [#11](/github-metadata-backup-bitcoin-bitcoin/11/) 0x559144915aed in LLVMFuzzerTestOneInput /workdir/bitcoin/src/test/fuzz/fuzz.cpp:214:5
        [#12](/github-metadata-backup-bitcoin-bitcoin/12/) 0x5591443b02eb in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:619:13
        [#13](/github-metadata-backup-bitcoin-bitcoin/13/) 0x559144399f92 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:329:6
        [#14](/github-metadata-backup-bitcoin-bitcoin/14/) 0x55914439fe30 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:864:9
        [#15](/github-metadata-backup-bitcoin-bitcoin/15/) 0x5591443ccb02 in main /llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
        [#16](/github-metadata-backup-bitcoin-bitcoin/16/) 0x7ff09eb61ca7  (/lib/x86_64-linux-gnu/libc.so.6+0x29ca7) (BuildId: 58749c528985eab03e6700ebc1469fa50aa41219)
        [#17](/github-metadata-backup-bitcoin-bitcoin/17/) 0x7ff09eb61d64 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29d64) (BuildId: 58749c528985eab03e6700ebc1469fa50aa41219)
        [#18](/github-metadata-backup-bitcoin-bitcoin/18/) 0x559144392f80 in _start (/workdir/out/libfuzzer_ubsan/fuzz+0x1745f80)
    
    NOTE: libFuzzer has rudimentary signal handlers.
          Combine libFuzzer with AddressSanitizer or similar for better crash reports.
    SUMMARY: libFuzzer: deadly signal
    

    </details>

  11. in src/test/fuzz/connman.cpp:231 in dfd3744b7b
     262 | @@ -228,20 +263,26 @@ FUZZ_TARGET(connman, .init = initialize_connman)
     263 |                  connman.SocketHandlerPublic();
     264 |              });
     265 |      }
     266 | -    (void)connman.GetAddedNodeInfo(fuzzed_data_provider.ConsumeBool());
    


    frankomosh commented at 10:04 AM on May 14, 2026:

    Noticed that include_connected=false path in GetAddedNodeInfo was uncovered was uncovered before this PR and remains so after. Checked coverage against existing corpus, and lines 2976 - 2977 and 2987 - 2988 in net.cpp have not been reached.

    Tracing through the code, the gap seems structural, nodes added via AddTestNode get fuzzed CAddress values while AddNode takes arbitrary strings, so mapConnectedByName is never populated relative to m_added_node_params. If it's worth addressing, I think collecting m_addr_name from test nodes during setup and reusing those names in the AddNode lambda could reach these paths?

    <details> <summary>Suggested fix</summary>

    @@ -109,12 +109,14 @@
    CSubNet random_subnet;
    std::string random_string;
    + std::vector<std::string> node_addr_names;
    
    LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 100) {
    CNode& p2p_node{*ConsumeNodeAsUniquePtr(fuzzed_data_provider).release()};
    p2p_node.fSuccessfullyConnected = true;
    connman.AddTestNode(p2p_node);
    + node_addr_names.push_back(p2p_node.m_addr_name);
    }
    
    @@ -133,14 +135,17 @@
    [&] {
    + const std::string& node_str = (!node_addr_names.empty() && fuzzed_data_provider.ConsumeBool())
    + ? PickValue(fuzzed_data_provider, node_addr_names)
    + : random_string;
    const auto added_node_info{connman.GetAddedNodeInfo(true)};
    - const auto add_node{connman.AddNode({random_string, fuzzed_data_provider.ConsumeBool()})};
    + const auto add_node{connman.AddNode({node_str, fuzzed_data_provider.ConsumeBool()})};
    if (add_node) {
    - assert(!connman.AddNode({random_string, fuzzed_data_provider.ConsumeBool()}));
    + assert(!connman.AddNode({node_str, fuzzed_data_provider.ConsumeBool()}));
    ...
    - assert(connman.RemoveAddedNode(random_string));
    + assert(connman.RemoveAddedNode(node_str));
    ...
    }
    
    @@ -263,6 +268,7 @@
    }
    + (void)connman.GetAddedNodeInfo(false);
    (void)connman.GetExtraFullOutboundCount();
    

    </details>

    Definitely not a blocker, but might be a solution to a structural coverage gap for this harness?


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:51 UTC