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

inprocessfork executor #237

Merged
merged 20 commits into from
Aug 7, 2021
Merged

inprocessfork executor #237

merged 20 commits into from
Aug 7, 2021

Conversation

tokatoka
Copy link
Member

#82

@tokatoka
Copy link
Member Author

tokatoka commented Jul 25, 2021

I had to change __sanitizer_cov_trace_pc_guard a bit to make it use a pointer instead of a static array, is this ok?
(
with this change, users can change the edges map later in the fuzzer like

+    let mut shmem;
+    unsafe{
+        shmem = StdShMemProvider::new().unwrap().new_map(MAX_EDGES_NUM).unwrap();
+    }
+    let shmem_map = shmem.map_mut();
+    unsafe{
+        EDGES_PTR = shmem_map.as_ptr();
+    }

)

@tokatoka tokatoka requested a review from domenukk July 25, 2021 07:11
@domenukk
Copy link
Member

Anyway, looks good to me, thanks! :) @andreafioraldi any objections?

@andreafioraldi
Copy link
Member

Anyway, looks good to me, thanks! :) @andreafioraldi any objections?

dunno, i'm sick today

@tokatoka
Copy link
Member Author

now we check if the EDGES_MAP_PTR is null, and if so we set EDGES_MAP in libfuzzer_initialize()
what do you think?

@andreafioraldi
Copy link
Member

now we check if the EDGES_MAP_PTR is null, and if so we set EDGES_MAP in libfuzzer_initialize()
what do you think?

No, edge coverage can be used even when there is no need to use the libfuzzer compatibility layer

@andreafioraldi
Copy link
Member

IMO, here the solution is to use a feature flag in libafl_targets. in-process fuzzing does not need the double dereference of EDGES_PTR, it is just wasted cpu time. Do not use libfuzzer_initialize but instead lazy_static (assuming that we need std for our inprocessfork feature flag, that we does because of fork())

@domenukk
Copy link
Member

Can't we just initialize the value when creating the Observer for the first time?

@andreafioraldi
Copy link
Member

Can't we just initialize the value when creating the Observer for the first time?

No, because a MapObserver is independent from that. We can use __sanitizer_cov_trace_pc_guard_init

@tokatoka
Copy link
Member Author

tokatoka commented Aug 6, 2021

yes I can use __sanitizer_cov_trace_pc_guard_init instead of libfuzzer_initialize
(the rust compiler complains with lazy_static!)

@domenukk
Copy link
Member

domenukk commented Aug 7, 2021

Should we add a #[test] doing a single exec to make sure this works?

libafl/src/executors/inprocess.rs Show resolved Hide resolved
libafl/src/executors/inprocess.rs Show resolved Hide resolved
@tokatoka
Copy link
Member Author

tokatoka commented Aug 7, 2021

Or maybe we just make it linux only for now?

sure :>

@domenukk
Copy link
Member

domenukk commented Aug 7, 2021

Or maybe we just make it linux only for now?

sure :>

Don't think that's really needed, unless we find bugs :D

@tokatoka
Copy link
Member Author

tokatoka commented Aug 7, 2021

then I'll leave pre/post_fork as they are

@domenukk domenukk merged commit 7f4e341 into main Aug 7, 2021
@tokatoka tokatoka deleted the inprocessfork branch August 7, 2021 10:15
khang06 pushed a commit to khang06/LibAFL that referenced this pull request Oct 11, 2022
* inprocessfork executor

* fmt

* cfg

* no_std

* no volatile rw

* wrapping_add

* fix

* mutable pointer

* ptr initialization in __sanitizer_cov_trace_pc_guard_init

* features

* more cfg

* fmt

* fix

* fmt

* post_fork

* fmt

* pre_fork

* test

* cfg
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.

3 participants