search

google / oss-fuzz

OSS-Fuzz - continuous fuzzing for open source software.
https://google.github.io/oss-fuzz
Apache License 2.0
10.36k stars 2.2k forks source link

CIFuzz should run all fuzz targets when all of them aren't "affected" #7195

Closed evverx closed 2 years ago

evverx commented 2 years ago

In https://github.com/systemd/systemd/runs/4974919673?check_suite_focus=true CIFuzz ran just one fuzz target even though all of them weren't affected (due to https://github.com/google/oss-fuzz/issues/7011). My guess would be that it happened because that particular fuzz target was added yesterday and it somehow affected CIFuzz so instead of something like

2022-01-26 19:18:18,922 - root - INFO - No affected fuzz targets detected, keeping all as fallback.

it decided to treat only that one fuzz target as affected

2022-01-28 01:19:54,058 - urllib3.connectionpool - DEBUG - https://storage.googleapis.com:443 "GET /oss-fuzz-coverage/systemd/fuzzer_stats/20220127/fuzz-dhcp-server-relay-message.json HTTP/1.1" 200 189572
2022-01-28 01:19:54,084 - root - INFO - Could not get coverage for fuzz-dhcp-server-relay-message. Treating as affected.
...
2022-01-28T01:20:02.8416671Z 2022-01-28 01:20:02,840 - root - INFO - Removing unaffected fuzz targets: {'/github/workspace/build-out/fuzz-env-file', '/github/workspace/build-out/fuzz-netdev-parser', '/github/workspace/build-out/fuzz-link-parser', '/github/workspace/build-out/fuzz-hostname-setup', '/github/workspace/build-out/fuzz-udev-rule-parse-value', '/github/workspace/build-out/fuzz-journald-stream', '/github/workspace/build-out/fuzz-network-parser', '/github/workspace/build-out/fuzz-lldp-rx', '/github/workspace/build-out/fuzz-nspawn-settings', '/github/workspace/build-out/fuzz-journald-syslog', '/github/workspace/build-out/fuzz-systemctl-parse-argv', '/github/workspace/build-out/fuzz-udev-database', '/github/workspace/build-out/fuzz-unit-file', '/github/workspace/build-out/fuzz-xdg-desktop', '/github/workspace/build-out/fuzz-journald-kmsg', '/github/workspace/build-out/fuzz-bus-message', '/github/workspace/build-out/fuzz-catalog', '/github/workspace/build-out/fuzz-bus-label', '/github/workspace/build-out/fuzz-dhcp-server', '/github/workspace/build-out/fuzz-calendarspec', '/github/workspace/build-out/fuzz-bcd', '/github/workspace/build-out/fuzz-dhcp6-client', '/github/workspace/build-out/fuzz-compress', '/github/workspace/build-out/fuzz-varlink', '/github/workspace/build-out/fuzz-nspawn-oci', '/github/workspace/build-out/fuzz-fido-id-desc', '/github/workspace/build-out/fuzz-journald-native-fd', '/github/workspace/build-out/fuzz-journald-native', '/github/workspace/build-out/fuzz-bus-match', '/github/workspace/build-out/fuzz-time-util', '/github/workspace/build-out/fuzz-journal-remote', '/github/workspace/build-out/fuzz-json', '/github/workspace/build-out/fuzz-ndisc-rs', '/github/workspace/build-out/fuzz-udev-rules', '/github/workspace/build-out/fuzz-dns-packet', '/github/workspace/build-out/fuzz-journald-audit', '/github/workspace/build-out/fuzz-etc-hosts'}.
2022-01-28T01:20:06.3278702Z 2022-01-28 01:20:06,327 - root - INFO - Build check passed.
2022-01-28T01:20:06.3279370Z Build check: stdout: INFO: performing bad build checks for /tmp/not-out/tmpq8a7wn4q/fuzz-dhcp-server-relay-message
evverx commented 2 years ago

At this point CIFuzz doesn't seem to be running anything apart from that fuzz target so to really test PRs the fuzz targets have to be built and run manually.

evverx commented 2 years ago

I think due to this and broken coverage it would make sense to replace CIFuzz with CFLite. It runs all fuzz targets predictably at least

jonathanmetzman commented 2 years ago

I think this is an interesting edge case. Basically, if we cant get coverage for a fuzz target, we treat it as affected. If no target is affected, we keep all targets (as you would desire). However, if one fuzz target is treated as affected because we can't get its coverage, then we delete all other targets. Fixing this will probably be ugly, unsure if it's worth the complexity.

jonathanmetzman commented 2 years ago

I think due to this and broken coverage it would make sense to replace CIFuzz with CFLite. It runs all fuzz targets predictably at least

I plan on fixing the broken coverage. Maybe I'll get a chance today.

evverx commented 2 years ago

I agree it's a corner case but due to it I basically had to build and run the fuzzers manually for about a week because two new fuzz targets were introduced. Interestingly since due to those issues I kind of kept track of what's going on on CIFuzz I ran into another bug where timeouts were reported even though by default CIFuzz isn't supposed to report them. In that particular case it was helpful because it caught a bug in a fuzzer but generally I don't think -timeout 25 should be passed when REPORT_TIMEOUTS is false. It was briefly discussed in https://github.com/google/oss-fuzz/pull/6711#issuecomment-955775337 as far as I can remember.

evverx commented 2 years ago

I don't think -timeout 25 should be passed when REPORT_TIMEOUTS is false

Given that the last four timeouts caught by CIFuzz/CFLite I've seen were actual bugs in systemd/elfutils I think I'll just set REPORT_TIMEOUTS to True to catch them explicitly. It has nothing to do with affected fuzz targets though :-)

evverx commented 2 years ago

systemd started running all fuzz targets unconditionally with keep-unaffected-fuzz-targets so I think the issue can be closed (assuming it can't be fixed).