Skip to content

Fuzzing via cargo-fuzz, take 2 - #1483

Open
Shnatsel wants to merge 4 commits into
memorysafety:mainfrom
Shnatsel:fuzzing2
Open

Shnatsel wants to merge 4 commits into
memorysafety:mainfrom
Shnatsel:fuzzing2

Conversation

@Shnatsel

@Shnatsel Shnatsel commented Apr 4, 2026

Copy link
Copy Markdown
Contributor
  1. Add a fuzzing harness using cargo fuzz
  2. Expose a Rust API to disable logging, which slows down fuzzing significantly

This is essentially #1475 but rebased on top of main. It has already discovered an issue: #1476

@Shnatsel

Shnatsel commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

No issues other than #1484 discovered after 75 million iterations

@thedataking thedataking left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good PR for initial fuzzing support. Thanks!

I think this can land once a few minor items have been considered.

nit: could we rename fuzz/fuzz_targets/fuzz_target_1.rs to something like fuzz/fuzz_targets/decoder.rs?

Comment thread fuzz/Cargo.toml

[dependencies.rav1d]
path = ".."
default-features = false # Disables ASM; comment out to enable

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does fuzzing not work with asm enabled? or is this to get coverage on the non-assembly versions?

In practice, assembly is always enabled as speed is otherwise unacceptable and I imagine it would speed up fuzzing too.

If this has to stay off, could you document the reason?

Comment on lines +33 to +36
let mut settings = rav1d::rust_api::Settings::new();
settings.set_logger_enabled(false);
settings.set_frame_size_limit(256 * 256); // in pixels, 256x256 image max
let mut dec = rav1d::rust_api::Decoder::with_settings(&settings).expect("failed to create decoder instance");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we bound the worker count? I think you get one worker per logical CPU.
My worry is that this could cause a slowdown if someone runs multiple parallel fuzz jobs.

Comment on lines +62 to +63
// Handle all pending pictures before sending the next data.
handle_pending_pictures(&mut dec, false)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add an end-of-input drain after this call? send_data() accepting the buffer doesn’t mean decoding is finished, and a single get_picture() can return TryAgain while threaded decoding is still pending, or return only the first of several pictures.

This could let the fuzzer cover more of the decoder logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants