Add Stoppable trait to State which exposes an API to stop the fuzzer

[R9295]

No description provided.

[domenukk]

I'm not sure about the name. How about should_stop_fuzzing or similar?

[R9295]

Yeah, I wanted to keep it in line with AFL++ but I'll change it

[domenukk]

clippy complains.

error[E0277]: the trait bound `S: libafl::state::HasShouldStopFuzzing` is not satisfied
   --> src/fuzz.rs:68:8
    |
68  |     F: Fuzzer<E, EM, ST, State = S>,
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `libafl::state::HasShouldStopFuzzing` is not implemented for `S`

Also, should the trait just be called Stoppable or something?

[R9295]

We can do stoppable, but Has... is more inline with other traits.

[R9295]

@domenukk I was also thinking of introducing a new Event::Stop for multi-threaded fuzzing. What do you think?

[R9295]

And also adding this to load_file & generate_initial_internal so seed loading can be stopped.

[domenukk]

We can do stoppable, but Has... is more inline with other traits.

I was thinking we can reuse that trait for other parts like the stage. But maybe you're right.
I'll leave it up to you, to be aligned either let's do Stoppable and should_stop (no fuzzing) or go back to the Has version.

WRT Event::Stop, right now it works via signals but I can see it won't work for all setups, so yes let's do it.

[R9295]

No let's keep Stoppable then, I had not thought of it's re-use potential.

[R9295]

Hmm this is still crashing when using LlmpRestartingEventManager and firing Event::Stop from a Stage.

Fuzzer-respawner: Storing state in crashed fuzzer instance did not work, no point to spawn the next client! This can happen if the child calls `exit()`, in that case make sure it uses `abort()`, if it got killed unrecoverable (OOM), or if there is a bug in the fuzzer itself. (Child exited with: 0)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[domenukk]

Usually if you use mgr.on_restart(&mut state)?; in order to store the state or mgr.send_exiting()?; the respawner shouldn't complain. Are you sure you are actually calling send_exiting?

[rmalmain]

as far as i understand the stop event is not created at all so far, why would you need to ask for stopping the fuzzer from the state?
also there is #2334 that is kind of related. once we received a stop signal, we store that we wish to stop fuzzing and we exit at the end of the fuzzing run to avoid state corruption, so that we can dump it on disk. the thing is we cannot access the state struct from the signal handler (with the current implementation at least), so i guess the goal is different?

[domenukk]

as far as i understand the stop event is not created at all so far, why would you need to ask for stopping the fuzzer from the state? also there is #2334 that is kind of related. once we received a stop signal, we store that we wish to stop fuzzing and we exit at the end of the fuzzing run to avoid state corruption, so that we can dump it on disk. the thing is we cannot access the state struct from the signal handler (with the current implementation at least), so i guess the goal is different?

The Stoppable trait would be the thing that stops fuzzing from a signal handler for example, the event is just a bonus to stop opther nodes as well (optionally)
So I think you concurrently implemented the same thing? :D

[rmalmain]

I don't think so, we don't have access to the state from the signal handler. i don't think it is easy to fix, since we could have a signal happening while we are mutating the state, so we cannot use static mut (except if we use synchronization primitives, but we would have an overhead for each access to the state, which is not something we want i believe)

[domenukk]

Ah right, I agree - we definitely need to set the flag outside of the state. Both PRs should be merged somehow

[domenukk]

Shouldn't the run loop already reutrn ShuttingDown?

[R9295]

Nope

[domenukk]

Why not? Also, can you change noed -> node in the main_run_client fn docu while you're having a PR open? :D

[R9295]

where is this? I can't seem to find it.

[R9295]

It is not necessary that the run_client function propagates Error::ShuttingDown, it may decide to handle it manually and return Ok(()), so we need to force return an Error::ShuttingDown

[R9295]

@domenukk we should be good now

[domenukk]

Note that I still prefer cancel :P