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

Simplify Executor #2897

Open
riesentoaster opened this issue Jan 27, 2025 · 5 comments
Open

Simplify Executor #2897

riesentoaster opened this issue Jan 27, 2025 · 5 comments
Labels
enhancement New feature or request

Comments

@riesentoaster
Copy link
Contributor

The Executor trait currently gets 2 parameters that are only ever used in the crash handlers of InProcessExecutor and probably shouldn't actually be touched by any executor: objectives and event_mgr. However, the crash handlers need access to them. We should restructure LibAFL in a way where these parameters do not need to be passed to the executor.

Discussion started in #2892, and some approaches were discussed there already.

@tokatoka
Copy link
Member

However, the crash handlers need access to them. We should restructure LibAFL in a way where these parameters do not need to be passed to the executor.

The problem is that these pointers has to be passed right before harness function is called.
and that "right before" is exactly somewhere inside run_target.

so i think removing them from run_target is not possible

@riesentoaster
Copy link
Contributor Author

What I'm suggesting is splitting StdFuzzer into multiple parts. The most important is going to be an implementation of Evaluator, which will own feedbacks and objectives. Then, we can have multiple (wrapped!) Evaluators, one of them can allow the Executor execution to panic. So I would set the pointers just before calling run_target, which should be close enough for restarting the fuzzing client.

Plus it would mean that all interaction with objectives is done in one place (namely the evaluators).

Filtering and replaying evaluators could then probably just be implemented as wrappers, similar to e.g. monitors.

@tokatoka
Copy link
Member

tokatoka commented Jan 29, 2025

Sorry i still don't understand the design.

So I would set the pointers just before calling run_target

Doesn't this mean you still have to pass Evaluator into the run_target?

@riesentoaster
Copy link
Contributor Author

riesentoaster commented Jan 29, 2025

No, the other way around.

I'd imagine extending the Executor trait to something like this:

pub trait Executor<I, OT, S> {
    fn run_target(&mut self, input: &I, observers: &mut OT) -> Result<ExitKind, Error>
    fn reset_observers_and_run_target(&mut self, input &I, observers: &mut OT, state: &mut S) -> Result<ExitKind, Error> {
        // default impl, may have more logic, this is just demonstration
        *state.executions_mut() += 1;
        observers.pre_exec_all(state, input)?;
        let res = self.run_target(input, observers);
        observers.post_exec_all(state, input)?;
        res
    }
}

Implementations can own handles to the observers to add data to them (e.g. StdOutObserver), but don't need to worry about state interactions since that's done in a default function.

Then, you have an Evaluator trait, which would be something like this (this is definitely not a finished thought):

pub trait Evaluator<EM, I, OT, S> {
    fn evaluate(
        &mut self,
        input: I,
        manager: &mut EM,
        state: &mut S,
    ) -> Result<Option<CorpusId>, Error>
}

The Evaluator implementation owns the executor, observers, feedbacks and objectives. It essentially first calls reset_observers_and_run_target on the executor and then checks to see if the input should be added to the corpus or the solutions. Besides, it appends metadata and whatever else we find necessary.

Stages would receive the evaluator (besides the state, manager, etc.)

This way, Executor implementations only need to worry about the target and filling target-specific Observers, while Evaluator worries about interactions with the corpus/feedbacks/objectives (and thus state).

There would also be an Evaluator implementation (maybe even in the StdEvaluator) that sets the hooks currently set in InProcessExecutor, since it already has access to everything it needs, and since all interactions with feedbacks/objectives/state are already happening there. Right now, they are in StdFuzzer, except for InProcessExecutor, where for some reason it is bodged onto the Executor.

This is still very much work in progress, we may find that we need other functionality.

@tokatoka
Copy link
Member

ok now i understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants