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

ForkserverExecutor #111

Merged
merged 54 commits into from
May 25, 2021
Merged

ForkserverExecutor #111

merged 54 commits into from
May 25, 2021

Conversation

tokatoka
Copy link
Member

@tokatoka tokatoka commented May 19, 2021

#82

Only the ForkserverExecutor, still WIP
TODO:

  • use Pipe made by Dominik.
  • We need to check if the execution has crashed
  • We might want to allocate the shared memory (calling StdShMemProvider::new()) outside the Executor?, because the Observer needs that address.
  • Add an example that uses ForkserverExecutor

@tokatoka
Copy link
Member Author

ehm, the commit log went messy when I rebased forkserver onto the main branch...

.setlimit(memlimit)
.setsid()
.setstdin(out_filefd, use_stdin)
.setpipe(st_pipe.clone(), ctl_pipe.clone())
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need Arc here, read and write ends are independent of each other

Copy link
Member Author

@tokatoka tokatoka May 19, 2021

Choose a reason for hiding this comment

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

but if we don't wrap it in Arc then Pipe's drop() would be prematurely called after setpipe, and following communication through the ctl_pipe and st_pipe won't work.
(and we can't pass normal reference & to Pipe either)

Copy link
Member Author

@tokatoka tokatoka May 19, 2021

Choose a reason for hiding this comment

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

ah, ok.
I guess it would simpler to pass 4 RawFds (st_pipe.read_end, st_pipe.write_end, ctl_pipe.read_end, ctl_pipe.write_end) to setpipe, then no Arc is needed

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it on drop the pipe in the parent after the child got spawned, in which case it is fine, though? Or does it drop before exec, too?
Maybe we need to ManuallyDrop, then?
In any case we shouldn't need Arc since we don't have more than 1 access to each side of the pipe I think (worst case, Rc?)

Copy link
Member Author

Choose a reason for hiding this comment

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

it drops after the forkserver is spawned (so after the match Command::new(target) line) for the parent,

        unsafe {
            libc::close(ctl_pipe.read_end);
            libc::close(st_pipe.write_end);
        }

these 2 are safe to close for the parent, but if the Pipe::drop() is called here, the remaining two ends (ctl_pipe.write_end, st_pipe.read_end) would also be closed, and we don't want that.
what we want instead is calling Pipe::drop() when the forkserver dies.
I'll look into ManuallyDrop.
I tried to use Rc, but pre_exec in std::os::unix::process::CommandExt wants std::marker::Send, so it didn't work.

Copy link
Member

Choose a reason for hiding this comment

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

We can change the Pipe interface to anything that makes sense.
I feel like we should move out the two ends for the forkserver, so they get dropped in the parent, and keep the respective other ends around in the fs parent, right?


fn setpipe(&mut self, st_pipe: Arc<Pipe>, ctl_pipe: Arc<Pipe>) -> &mut Self {
let func = move || {
let ret = unsafe { libc::dup2(ctl_pipe.read_end, FORKSRV_FD) };
Copy link
Member

Choose a reason for hiding this comment

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

We have a wrapper for dup2 in bolts::os.

use crate::inputs::NopInput;
#[test]
fn test_forkserver() {
let command = "/home/toka/work/aflsimple/test";
Copy link
Member

Choose a reason for hiding this comment

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

Would be cool to have a testcase, but this one can't work, sadly :)

Copy link
Member Author

@tokatoka tokatoka May 19, 2021

Choose a reason for hiding this comment

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

Yes 😄, I think I'll add a simple example binary file and a c source code file along with a example fuzzer.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should add a libafl-tests project that can have more complex testcases, such as a short fuzz session, etc.
Committing binaries will break on various platforms, and in various colors. :)

@andreafioraldi andreafioraldi mentioned this pull request May 22, 2021
10 tasks
@tokatoka
Copy link
Member Author

tokatoka commented May 23, 2021

ah.. I'm sorry, I did not notice that the test code hung up.. (and it has run for around 80 minutes..)

@tokatoka
Copy link
Member Author

tokatoka commented May 25, 2021

Hi, thanks for the help andrea!

@tokatoka is ~3k exec/sec the speed that you was expecting?

Yes I was expecting around 3k exec/sec.
but with this commit e772742, the fuzzer forkserver_simple still does not run as fast as AFL/afl-fuzz when it is run with cargo run on my computer,
(still around 200 exec/sec after several minutes)
am I missing some compiler options to build forkserver_test.o?

@andreafioraldi
Copy link
Member

Hi, thanks for the help andrea!

@tokatoka is ~3k exec/sec the speed that you was expecting?

Yes I was expecting around 3k exec/sec.
but with this commit e772742, the fuzzer forkserver_simple still does not run as fast as AFL/afl-fuzz when it is run with cargo run on my computer,
(still around 200 exec/sec after several minutes)
am I missing some compiler options to build forkserver_test.o?

--release

@tokatoka
Copy link
Member Author

--release

😲
ah, I'm so stupid...
OK!, now it is working as I am expecting.

@tokatoka tokatoka marked this pull request as ready for review May 25, 2021 10:05
@tokatoka
Copy link
Member Author

tokatoka commented May 25, 2021

I'd like to add a feature like forkserver_test for libafl_tests, and then we can modify build.rs so that we fetch AFL++, compile AFL++ and compile forkserver_test.c only when forkserver_test feature is enabled.
but I also want to make the test in forkserver.rs dependent on libafl_tests's forkserver_test feature (not libafl's feature!), (otherwise we don't have the binary file, forkserver_test.o).
What would be a good workaround for this?
(I think yes, we can just check libafl_tests/src/forkserver_test.o is existent for the forkserver.rs's test to work, but that looks somewhat ugly)

@domenukk
Copy link
Member

Just add minimial tests to forkserver.rs and do the "real" testing in libafl_tests. No need to test it two times.

@tokatoka tokatoka changed the title WIP: ForkserverExecutor ForkserverExecutor May 25, 2021
@andreafioraldi andreafioraldi merged commit d4410c0 into main May 25, 2021
@tokatoka tokatoka mentioned this pull request May 28, 2021
@tokatoka tokatoka deleted the forkserver branch June 2, 2021 07:19
khang06 pushed a commit to khang06/LibAFL that referenced this pull request Oct 11, 2022
* add Forkserver, Pipe Outfile struct

* add forkserver executor struct, and shmem init

* close pipes in the destructor of Forkserver

* fill pre_exec to write out the inputs

* fix

* read_st, write_ctl

* more handshakes

* wrap Pipe in Arc, fill post_exec

* add Forkserver, Pipe Outfile struct

* add forkserver executor struct, and shmem init

* close pipes in the destructor of Forkserver

* fill pre_exec to write out the inputs

* fix

* read_st, write_ctl

* more handshakes

* wrap Pipe in Arc, fill post_exec

* fix for the lastest HasExecHooks trait

* use Dominik's pipe, remove Arc and temporarily pass RawFd to setstdin but trying to figure out other solutions

* add libafl_tests, put a very simple vulnerable program

* fix

* added forkserver_simple (mostly copy-pasted from babyfuzzer)

* fix test

* handle crash in post_exec

* add README.md

* check exec time to see why it's so slow

* remove double invokation of is_interesting for the obejctive

* make forkserver_simple AFL-like and improve speed

* some debugging help

* do not evaluate feedback if solution

* speedup the things

* working input placement via stdin in Forkserver

* don't call panic! but return errors, rewrite some comments

* use AFLplusplus/afl-cc instead of AFL

* use .cur_input like AFL

* bring the test for forkserver back

* add better README.md message

* failing the initial handshake should return an error

* delete some commented-out code

* format

* format

* ForkserverExecutor needs std and is unix-only for now

* clippy

* OutFile error handling

* fmt

* clippy

* don't build libafl_tests on windows

* fix

* keep test in forkserver.rs simple

* add forkserver_test feature for libafl_tests

* format

* some doc

Co-authored-by: Andrea Fioraldi <[email protected]>
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