rust-fuzz / honggfuzz-rs

Fuzz your Rust code with Google-developed Honggfuzz !
https://crates.io/crates/honggfuzz
Apache License 2.0
449 stars 40 forks source link

Large fuzz target fails due to _HF_PC_GAURD_MAX being too small #19

Closed ghost closed 5 years ago

ghost commented 5 years ago

Our CI fuzzing recently broke as our fuzz target became too large so that honggfuzz would fail with errors like This process has too many PC guards. It seems this is based on a hard-coded constant _HF_PC_GUARD_MAX in the honggfuzz.h header which can only be changed when honggfuzz is built.

Hence, for now I resorted to adding

// increase _HF_PC_GUARD_MAX
let status = Command::new("sed")
    .args(&["-e", "s/^#define _HF_PC_GUARD_MAX .*/#define _HF_PC_GUARD_MAX (2U * 1024U * 1024U * 16U)/", "-i", "honggfuzz/honggfuzz.h"])
    .status()
    .expect("failed to patch hongfuzz.h using sed");
assert!(status.success());

to this crate's build script.

Would this be something you would accept upstream (controlled via an environment variable by the Cargo hfuzz subcommand)? Do you have any other ideas how to avoid this problem? Thank you for your help!

PaulGrandperrin commented 5 years ago

Hi @adam-rhebo, No problem we'll do something to solve your use-case. I'll just ping @robertswiecki (the upstream developer of the "real" honggfuzz code base) just to know if he has a better idea of how to implement that and if there's something that could be done directly in his code base :-)

robertswiecki commented 5 years ago

We can probably change it for everybody, is 32Mo guards sufficient for your tests?

ghost commented 5 years ago

We can probably change it for everybody, is 32Mo guards sufficient for your tests?

For now 2U * seems to suffice, but I only expect to add more code to that particular fuzz target, so I would be happy with 64 MB as well. But I admittedly do not understand the downsides of investing a larger amount of memory. Will performance suffer is the guards array becomes bigger?

robertswiecki commented 5 years ago

Unsure, it's probably large enough already that CPU caching doesn't matter much at this point, so maybe not.

robertswiecki commented 5 years ago

Please try at HEAD or with https://github.com/google/honggfuzz/commit/e2be7a962bad2ab1598b8ae1f55103968096c82c

ghost commented 5 years ago

Please try at HEAD or with google/honggfuzz@e2be7a9

Works like a charm and I am seeing no significant reduction fuzzing speed. Thank you very much for your swift help!

PaulGrandperrin commented 5 years ago

That's great! I'll pull that and issue a new release asap 🙂