fuzzers: limit input size to 100KiB - #16505
Merged
Merged
Conversation
While testing larger inputs can be useful for uncovering bugs related to processing large data or inefficiencies in memory allocation, it also makes the fuzzing process slower and more fragile. Beyond the obvious slowdown, there are OOM issues and even in libFuzzer. Specifically, in fuzzer::InputCorpus::AddToCorpus, after a few hours of fuzzing, we accumulate so many inputs that memory usage exceeds our quota. This could be mitigated in other ways, such as dumping data early or shortening the fuzzing run duration. However, in practice, these huge inputs are often not very useful. This change will also encourage the fuzzer to mutate existing data rather than continually adding more bytes to input, because it give higher coverage. Which in turn will produce higher quality corpus. The 100 KiB limit could be reduced further. It's still quite large, let's see how it performs. I believe even 10 KiB might be sufficient or less. This issue is especially noticeable in the Matroska fuzzer, because we add input data there, which is bigger and sets max_len high in fuzzer itself. In a perfect world, we wouldn't need to impose such limits, but in reality, we face constraints in both memory and compute resources. I'll monitor coverage and fuzzing results after this change and adjust as needed. Unfortunately, this will discard all existing corpus entries larger than the limit, but that's expected.
|
Download the artifacts for this pull request: Windows |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While testing larger inputs can be useful for uncovering bugs related to processing large data or inefficiencies in memory allocation, it also makes the fuzzing process slower and more fragile.
Beyond the obvious slowdown, there are OOM issues and even in libFuzzer. Specifically, in fuzzer::InputCorpus::AddToCorpus, after a few hours of fuzzing, we accumulate so many inputs that memory usage exceeds our quota. This could be mitigated in other ways, such as dumping data early or shortening the fuzzing run duration. However, in practice, these huge inputs are often not very useful.
This change will also encourage the fuzzer to mutate existing data rather than continually adding more bytes to input, because it give higher coverage. Which in turn will produce higher quality corpus.
The 100 KiB limit could be reduced further. It's still quite large, let's see how it performs. I believe even 10 KiB might be sufficient or less.
This issue is especially noticeable in the Matroska fuzzer, because we add input data there, which is bigger and sets max_len high in fuzzer itself.
In a perfect world, we wouldn't need to impose such limits, but in reality, we face constraints in both memory and compute resources.
I'll monitor coverage and fuzzing results after this change and adjust as needed. Unfortunately, this will discard all existing corpus entries larger than the limit, but that's expected.