Split into library and binary parts and implement fuzzer #74
Conversation
Cargo.toml
Outdated
| @@ -32,3 +32,7 @@ rstest = { version = "0.19.0", default-features = false } | |||
| rstest_reuse = "0.7.0" | |||
| tempfile = "3.0" | |||
| xattr = "1.3.1" | |||
|
|
|||
| [[bin]] | |||
| name = "fuse-ufs-fuser" | |||
There was a problem hiding this comment.
What does the "-fuser" part indicate? Simply "fuse-ufs" would be more consistent with precedent.
There was a problem hiding this comment.
There will be a fuse-ufs-fuse2 for OpenBSD in the future.
The right binary will be chosen when building with make and installed as fuse-ufs.
There was a problem hiding this comment.
Up until now, make hasn't been necessary for building; it's just been a convenience for development. It would be unfortunate if it suddenly becomes necessary. Plus, it would not be easy to integrate with things like the ports system. May I suggest two alternatives?
- Define a
fuserfeature which is on-by-default, but which must be turned off to build on OpenBSD. - Have a single
src/bin/fuse-ufsfile which is a thin wrapper around eithersrc/bin/fuse-ufs/fuser.rsorsrc/bin/fuse-ufs/fuse2,#[cfg()]-gated.
There was a problem hiding this comment.
I like this idea. I'll do this, once I have workspace-ified this project.
I think having fuser and fuse2 features would be useful, with only fuser being enabled by default.
On OpenBSD only fuse2 would work, on all other platforms, you could built with both (if you wanted that for some reason).
It sucks that per-platform features don't work
realchonk
changed the title
|
@asomers Feel free to review this now. |
| @@ -0,0 +1,13 @@ | |||
| #![cfg_attr(fuzzing, allow(dead_code, unused_imports, unused_mut))] | |||
There was a problem hiding this comment.
Do you really need to disable these lints for fuzzing? This looks more like something temporary that got left behind.
There was a problem hiding this comment.
These are required to silence warnings which occur due to parts of the code not being called while fuzzing.
There was a problem hiding this comment.
Better to remove those unused parts of code.
| ) | ||
| } | ||
| }; | ||
| // FIXME: Choose based on hash of input or so, to excercise BE as well with introducing non-determinism |
There was a problem hiding this comment.
I think that when fuzzing, you should guess the endianess just like you do when not fuzzing. You can generate multiple corpuses, right? One based on BE and one based on LE?
There was a problem hiding this comment.
Same issue as with the superblock check below - wasted fuzz cases due to guessing an invalid magic.
rufs/src/ufs/mod.rs
Outdated
| let mut file = Decoder::new(file, config); | ||
|
|
||
| let superblock: Superblock = file.decode_at(SBLOCK_UFS2 as u64)?; | ||
| #[cfg(not(fuzzing))] |
There was a problem hiding this comment.
Why disable this check when fuzzing?
There was a problem hiding this comment.
So the fuzzer doesn't have to guess a valid superblock, which would lead to a lot of wasted fuzz cases as they'd fail at this stage without reaching crashes/panics deeper in the code. It's common practice to disable these sorts of checks while fuzzing for this reason.
There was a problem hiding this comment.
The fuzzer doesn't generate totally random input, right? It starts from a valid disk image and then randomly mutates it?
There was a problem hiding this comment.
Given that it starts from a valid disk image, would you really expect to lost very many runs at this stage? And even if you disable the magic check here, wouldn't a run with a corrupt superblock still likely fail during SuperBlock::check ?
There was a problem hiding this comment.
This is no longer relevant, as I just always run the checks, even when fuzzing.
There was a problem hiding this comment.
Uh, what do you mean by "I just always run the checks"? Do you have a separate fork or something? Or is there a commit that you forgot to push? Because the magic check is still disabled during fuzzing.
There was a problem hiding this comment.
Oh, I forgot to remove this check. I did the match on the magic.
|
@asomers do you have any objections besides the (incomplete) fuzzing? |
TODO: