-
Notifications
You must be signed in to change notification settings - Fork 105
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
Add a fuzz! version that doesn't hook panics #154
Conversation
32cc1f8
to
3d564be
Compare
I'm interested in this feature as well - I'm planning a large-scale fuzzing run where we have little info about fuzz targets, so panics (i.e. indicating misuse of the API) is acceptable and expected, but segfaults are not. |
src/lib.rs
Outdated
|
||
let mut input = vec![]; | ||
|
||
// initialize forkserver there | ||
unsafe { __afl_manual_init() }; | ||
|
||
while unsafe { __afl_persistent_loop(1000) } != 0 { | ||
while unsafe { __afl_persistent_loop(1) } != 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pedrocr Do you remember why this change was made?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I forget why that was needed. Not sure if it just didn't work without that or if it was just because of performance. I can't find any docs for that function either to even know what the parameter does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just tested it and it works without this. I've pushed a version that rebased to master and doesn't have this change. The diff looks pretty clean now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know for sure, but I suspect that the parameter is how frequently the process is restarted during in-process fuzzing, in number of executions before restart. So if you set the parameter to 1 it should be noticeably slower, almost equivalent to the performance of stdin mode.
If your fuzzing code catches panics somewhere inside its code the hook would turn those into crashes. Allow disabling the hook by adding a fuzz_nohook! macro. Fixes rust-fuzz#150
a320b9d
to
3bbf439
Compare
/// # } | ||
/// ``` | ||
#[macro_export] | ||
macro_rules! fuzz_nohook { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fuzz_nohook!
is not a particularly readable name. How about fuzz_no_catch_panic!
or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me. Let me know what name you prefer.
Correction: this PR should actually be unrelated to what I'm trying to accomplish. The macro from this PR would not make the panic register as a crash, but still let panics terminate the process. My use case calls for catching panics and ignoring them without letting them terminate the process, since terminating the process would slow down fuzzing by a factor of up to 10x. I have (belatedly) realized that ignoring panics can actually be achieved by catching the panic inside the closure passed to |
@Shnatsel you need both this PR and to catch panic in your code. |
Thanks! |
If your fuzzing code catches panics somewhere inside its code the hook would turn those into crashes. Allow disabling the hook by adding a fuzz_nohook! macro.
Even in empty targets the hook causes variability in the execution of the code somehow. That reduces the stability of the fuzzing making it less effective.
Fixes #150