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

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

Closed
evverx opened this issue Jan 28, 2022 · 7 comments
Closed

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

evverx opened this issue Jan 28, 2022 · 7 comments

Comments

@evverx
Copy link
Contributor

evverx commented Jan 28, 2022

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 #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
Copy link
Contributor Author

evverx commented Jan 28, 2022

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
Copy link
Contributor Author

evverx commented Feb 9, 2022

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
Copy link
Contributor

jonathanmetzman commented Feb 9, 2022

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
Copy link
Contributor

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
Copy link
Contributor Author

evverx commented Feb 9, 2022

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 #6711 (comment) as far as I can remember.

@evverx
Copy link
Contributor Author

evverx commented Feb 9, 2022

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
Copy link
Contributor Author

evverx commented Feb 11, 2022

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

@evverx evverx closed this as completed Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants