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

oss-fuzz targets need improvement #198

Open
ramosian-glider opened this issue Oct 4, 2024 · 2 comments
Open

oss-fuzz targets need improvement #198

ramosian-glider opened this issue Oct 4, 2024 · 2 comments

Comments

@ramosian-glider
Copy link

The existing libFuzzer targets used by oss-fuzz may require some massage. I was looking at ./tests/fuzz/ext2fs_image_read_write_fuzzer built with ./configure --enable-fuzzing --enable-addrsan, and want to share my findings.

  1. Running ext2fs_image_read_write_fuzzer produces a lot of warnings like "Attempt to read block from filesystem resulted in short read while trying to open file system". These are not needed during fuzzing, is it possible to suppress them (e.g. by using a special error table with empty strings?)
  2. At some point the test dies with OOM, with memory leaking from add_error_table(). We need to properly tear it down by calling remove_error_table(&et_ext2_error_table) at the end of LLVMFuzzerTestOneInput().
  3. At startup, ext2fs_image_read_write_fuzzer reports there are 203 coverage counters in the binary, which is suspiciously few. My understanding is that by default coverage instrumentation is only added to tests/fuzz, and e.g. lib/ is left uninstrumented. As a result, the fuzzer is mostly blindfolded. When I manually update the Makefiles to pass -fsanitize=fuzzer at compile time1, the number of coverage counters in the resulting binary goes up to 6K+.
  4. Even after this, the coverage reported by libFuzzer on local runs doesn't go above 191. I think we must be hitting some sort of CRC check, which is also confirmed by the lib/ext2fs coverage report from the oss-fuzz dashboard. I am not completely sure though, how oss-fuzz notices coverage in lib/ext2fs/crc16.c, which is also not instrumented. It might be a good idea to provide a debug config disabling checksums, or teach the fuzzer to recalculate them from the modified binary.

Footnotes

  1. Proper implementation requires some plumbing, because binaries that are not fuzz targets cannot be linked with -fsanitize=fuzzer.

@tytso
Copy link
Owner

tytso commented Oct 4, 2024

The oss-fuzz was originally written by a intern, and I'm really not all that familiar with how the oss-fuzz project builds the fuzzer. I added it to e2fsprogs just to make ti easier for me to try to work oss-fuzz bugs. So I don't really know how to deal with (3).

As far as (1) is concerned, why are the warnings a problem?

@ramosian-glider
Copy link
Author

Warnings aren't a problem per se, but they aren't useful either. They can also slow down the fuzzing process, which is unfortunate when fuzzing locally. For oss-fuzz it is probably less of an issue, because they have way more machine time.

But a more important thing is that the fuzzer does not gain new coverage over time. Does the report linked above look representative to you, or maybe we are indeed hitting some roadblock?

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