Introduce libafl-fuzz #2362
Introduce libafl-fuzz #2362
Conversation
|
should we really put this into fuzzers/ ? |
|
it could be in a separate repo |
|
Yes otherwise it won't be maintained |
| @@ -0,0 +1,246 @@ | |||
| #![feature(io_error_more)] | |||
| #![deny(clippy::pedantic)] | |||
| #![allow(clippy::unsafe_derive_deserialize)] | |||
There was a problem hiding this comment.
Copy&paste the massive amount of flags we use in libafl/lib.rs
There was a problem hiding this comment.
Why? I would rather add it only if necessary and so far it hasn't been
There was a problem hiding this comment.
You'll be forced to write cleaner code through annoying lints
There was a problem hiding this comment.
clean code is good 🫡
| fn main() { | ||
| env_logger::init(); | ||
| let mut opt = Opt::parse(); | ||
| executor::check_binary(&mut opt, SHMEM_ENV_VAR).expect("binary to be valid"); |
There was a problem hiding this comment.
Pass in the binary into the function instead of the opt, and give some useful output on failure
There was a problem hiding this comment.
opt is necessary since there are several checks that may be skipped if frida, qemu, non instrumented, skip bin check etc
There was a problem hiding this comment.
Sad. Would still be good to pass in the needed flags only, but up to you. At least make the expect more meaningful?
There was a problem hiding this comment.
The errors returned from check_binary will also be printed when the expect is triggered so should be fine.
fuzzers/libafl-fuzz/src/afl_stats.rs
Outdated
| /// The [`AflStatsStage`] is a Stage that calculates and writes | ||
| /// AFL++'s `fuzzer_stats` and `plot_data` information. | ||
| #[derive(Debug, Clone)] | ||
| pub struct AflStatsStage<E, EM, Z> |
There was a problem hiding this comment.
Think we could put that into the main lib
|
Looks good! |
|
Post GSoC I think it makes sense to move it out of fuzzers. I would be happy to maintain it:) |
|
CI isn't happy yet, a bunch of warnings and an error |
| } | ||
| impl Display for AFLFuzzerStats<'_> { | ||
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| writeln!(f, "start_time : {}", &self.start_time)?; |
There was a problem hiding this comment.
I wonder if it's faster to format this and then do one big write, since every writeln may trigger a syscall
There was a problem hiding this comment.
As in, we coudl make this one a big
writeln!(f, "...
...
...
", ...)- uglier to read but potentially faster(?)
| @@ -55,6 +58,12 @@ where | |||
| .map(|id| state.corpus().next(id)) | |||
| .flatten() | |||
| .unwrap_or_else(|| state.corpus().first().unwrap()); | |||
|
|
|||
| self.runs_in_current_cycle += 1; | |||
| // TODO deal with corpus_counts decreasing due to removals | |||
There was a problem hiding this comment.
Can we already merge this anyway?
cc @tokatoka
There was a problem hiding this comment.
IIRC yes since WeightedScheduler also handles it this way. Also TODO there
| load_callback, | ||
| } | ||
| } | ||
|
|
||
| #[allow(clippy::only_used_in_recursion)] |
There was a problem hiding this comment.
error: parameter is only used in recursion
--> libafl/src/stages/sync.rs:176:10
|
176 | &self,
| ^^^^
|
note: parameter used here
--> libafl/src/stages/sync.rs:203:40
|
203 | let dir_left_to_sync = self.load_from_directory(&entry.path(), last)?;
| ^^^^
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#only_used_in_recursion
note: the lint level is defined here
--> libafl/src/lib.rs:18:9
|
18 | #![deny(clippy::all)]
| ^^^^^^^^^^^
= note: `#[deny(clippy::only_used_in_recursion)]` implied by `#[deny(clippy::all)]`There was a problem hiding this comment.
Ok technically load_from_directory could be a normal fn instead of being a method
There was a problem hiding this comment.
Take a look at #2402 what you think
|
Congrats 🎉🎉🎉 |
| .left_to_sync | ||
| .append(&mut new_files); | ||
| if let Some(last) = last { | ||
| if (last + self.interval) < current_time() { |
There was a problem hiding this comment.
Isn't this the opposite?
You want to perform this stage once every while,
shouldn't this be current_time() < (last + self.interval)
There was a problem hiding this comment.
Yeah, that's why it returns Ok(()) if last + self.interval < current_time
https://github.com/R9295/LibAFL/blob/d9ca434207e1828b7512e5046175ed3a8c33f307/libafl/src/stages/sync.rs#L101-L104
There was a problem hiding this comment.
But instead of the early return, I can wrap it in a different clause
There was a problem hiding this comment.
what i mean is if you want early return if not enough time has passed
then it should be
(last + self.interval) > current_time()
right?
There was a problem hiding this comment.
Ah yes, you're right. Whoops. I'll submit a fix
There was a problem hiding this comment.
yeah it's confusing
maybe do something like
let cur = current_time();
if cur.checked_sub(last) < self.last {
// early return
}
There's a few TODOs before we can merge this but I'd like to get some input on the work already.
test-instr.cdoes not work withForkserverExecutorsince it has an old forkserver version? Find something elsecc @vanhauser-thc / @domenukk / @tokatoka