Skip to content

Refactor to new forkserver #3183

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

Merged
merged 12 commits into from
May 5, 2025
Merged

Conversation

Evian-Zhang
Copy link
Contributor

@Evian-Zhang Evian-Zhang commented May 1, 2025

Description

Currently in the implementation of start_forkserver, the communication to AFL forkserver is coupled with communication with children. I'm improving unicornafl to better use LibAFL, and this makes it hard to custom communication between parent and children.

Moreover, I found the forkserver logic is written in C, which I didn't find some specific reason for that.

As a result, I use Rust to implement such logic (it's one-to-one rewrite, no logic change except for removing codes under USEMMAP macro that are never compiled), and add convenient trait for parent-children communication.

I'm poor at naming, so please correct me if I give name wrong, especially for forkserver-related names:

In my understanding, there are several entities in forkserver logic:

  • The main process part

    This part conduct the mutation, scheduling, etc., and both AFL++ (afl-fuzz executable) and LibAFL (ForkserverExecutor in libafl::executors) have implementations.

  • The parent process part

    This part communicate with main part with dedicated forkserver fd to indicate the process of each round. And it also in charge of spawning children and communicate with children. This part is where start_forkserver function should be invoked. ForkServerParent is used as a helper trait to communicate with children.

  • The child process part

    This part is forked by the parent process, and does the real execution and observation.

Checklist

  • I have run ./scripts/precommit.sh and addressed all comments

@Evian-Zhang
Copy link
Contributor Author

Evian-Zhang commented May 1, 2025

Accidentallu deleted some global variables, will fix it few hours later. Fixed

The clippy CI says something wrong in executors/forkserver.c, but that code I did not touch. Strange...

@Evian-Zhang Evian-Zhang marked this pull request as draft May 1, 2025 13:36
@Evian-Zhang
Copy link
Contributor Author

I will investigate into the broken pipe issue...

@wtdcode
Copy link
Member

wtdcode commented May 2, 2025

Does it surpass my #3087 ? If so I will take care of this. Just let me know when you are satisfied with the code.

@Evian-Zhang
Copy link
Contributor Author

Maybe yes. This PR enables us to conduct the fallback in downstream. If this PR got merged, I will further modify unicornafl to fit in. :)

@Evian-Zhang Evian-Zhang marked this pull request as ready for review May 2, 2025 02:27
@Evian-Zhang
Copy link
Contributor Author

Finally passed all tests... @wtdcode The code is ready for review. Thank you for your patience :)

@tokatoka
Copy link
Member

tokatoka commented May 2, 2025

can you do some benchmark comparing forkserver.rs and forkserver.c

…rkserver parent has not idea of whether it is persistent and the persistent version can handle the non-persistent version
@Evian-Zhang
Copy link
Contributor Author

can you do some benchmark comparing forkserver.rs and forkserver.c

Sure, I'll do it asap.

@Evian-Zhang
Copy link
Contributor Author

Tested fuzzers/forkserver_libafl_cc using just run with timeout 1h in Ubuntu 22.04 i9-13900K, the C version is 5.167K/s, the Rust version is 5.238K/s, which is acceptable for me :) (This PR is not primarily aimed at performance boosting; Instead, using Rust is to make it more generalized with help of trait system)

@tokatoka
Copy link
Member

tokatoka commented May 2, 2025

then it's ok

Copy link
Member

@wtdcode wtdcode left a comment

Choose a reason for hiding this comment

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

Very cool work. Thanks for your efforts =)

@wtdcode
Copy link
Member

wtdcode commented May 2, 2025

#3087 can be closed in favor of this I think.

@Evian-Zhang
Copy link
Contributor Author

Thank you @wtdcode :) I already have a worked version of unicornafl for this patch that still needs some cleanup. Please wait me for about 4-5 days (some other stuff occupies me) and I will send a patch.

@wtdcode
Copy link
Member

wtdcode commented May 2, 2025

No hurry, I'm on academic travel.

@Evian-Zhang
Copy link
Contributor Author

I have merged the latest commit in main branch, please notify me if there is anything I missed to get this PR accepted :)

@wtdcode wtdcode merged commit c0e32cd into AFLplusplus:main May 5, 2025
108 checks passed
@wtdcode
Copy link
Member

wtdcode commented May 5, 2025

Thanks =)

p13l13d13 pushed a commit to p13l13d13/LibAFL that referenced this pull request May 5, 2025
* Refactor to new forkserver

* Fix fuzzer examples and delete forkserver.c

* Fix clippy and doc warnings

* Fix symbol error

* Format Cargo.toml; Fix wrong doc link

* Fix silly typo.

* Rename ForkServer to Forkserver to make it more consistent

* Fix build.rs

* Merge StdForkserverParent and PersistentForkserverParent since the forkserver parent has not idea of whether it is persistent and the persistent version can handle the non-persistent version

* Fix clippy

* Do not take ownership for last_child_pid since it may be in persistent mode
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