Skip to content
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 an observer for COUNTERS_MAPS for 8-bit SanCov #1283

Merged
merged 11 commits into from
May 23, 2023

Conversation

novafacing
Copy link
Contributor

There is only really one way to observe the 8-bit sancov counter map, so I figured since we have std_edges_map_observer we might as well have a nice easy to use shortcut for doing the same for it.

/// `SanitizerCoverage` in [`COUNTERS_MAPS`]
#[derive(Serialize, Deserialize, Debug)]
#[allow(clippy::unsafe_derive_deserialize)]
pub struct CountersMultiMapObserver<const DIFFERENTIAL: bool> {
Copy link
Member

Choose a reason for hiding this comment

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

The observers should live in the main lib, not in libafl_targets (I think)

Copy link
Contributor Author

@novafacing novafacing May 19, 2023

Choose a reason for hiding this comment

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

Having libafl depend on libafl_targets to make the static visible would introduce a circular dependency, so I think it does have to go in here. I could be missing another option though.

(it can also go in your binary of course, but then I can't reuse this observer 😋)

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see, I thought the dependency went the other way.
But in this case, maybe we should hide additional observers behind a observers flag? Dragging in intervaltree and ahash for this seems excessive for most usecases, what do you think?

Copy link
Contributor Author

@novafacing novafacing May 23, 2023

Choose a reason for hiding this comment

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

Yeah libafl_targets is already full of feature flags, no biggie to add another :)

I pushed that change up adding an observers flag and moved the observer into there.

Checking over the source, I notice that cmplog.rs has CmpLogObserver and coverage.rs has DifferentialAFLMapSwapObserver (although it is already behind the "pointer_maps" flag). Probably not be worth gating the CmpLogObserver since it doesn't add any dependencies, but it might be worth gating all three the same way to trim down static library size and make things consistent.

One more note is that DifferentialAFLMapSwapObserver uses serde, but it's the only thing other than CountersMapsObserver that does. We should probably make serde an optional dependency as well (we could gate it behind "observers" if we add it as a feature gate for all the observers in the crate), because it has the "alloc" feature enabled which makes it not possible to use libafl_targets in no_std currently.

(This is probably another PR, happy to open it if these suggestions sound good)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. FWIW using alloc in no_std is totally possible. You just need to define an allocator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, today I learned!

@novafacing
Copy link
Contributor Author

novafacing commented May 19, 2023

I made counters_maps_observer #[no_mangle] to facilitate this use case:

  • Rust target compiled as staticlib, with libafl_targets as a dependency
  • Rust fuzzer linked statically with target lib

Basically, having it no_mangle avoids you needing to do something silly like:

use libafl_targets::counters_maps_observer as real_counters_maps_observer;
use libafl_targets::CountersMultiMapObserver;

#[export_name = "counters_maps_observer"]
pub fn counters_maps_observer(name: &'static str) -> CountersMultiMapObserver<false> {
    unsafe { real_counters_maps_observer(name) }
}

Edit: clippy doesn't like no_mangle for rust symbols, changed to #[export_name] which is basically equivalent here.

@domenukk domenukk merged commit 5a6d683 into AFLplusplus:main May 23, 2023
@novafacing novafacing deleted the novafacing/sancov-observer branch May 23, 2023 21:43
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