Non-zero exit code is considered ok in `CommandExecutor`

[momvart]

In CommandExecutor the exit code of the child is not checked and anything exited without a signal is counted as Ok.

https://github.com/AFLplusplus/LibAFL/blob/a8d2e8711bf735f3b6455f4173f5510bf4d04160/libafl/src/executors/command.rs#L357

Is it intentional? Is the executor only interested in crashes like segfaults? If so, shouldn't we add to the configurator? Because let's say a Rust program panics (with unwinding behavior), it will exit normally with a non-zero code, but something unexpected might have happened in it.

[tokatoka]

Is it intentional?

Yes. We consider it as a crash only if it was stopped by signal. because you can't really say which exit status (except for the signals) should be crash or not, that depends on your target.

Is the executor only interested in crashes like segfaults?

Not just segfaults, but any signals that stopeed the program from executing further including aborting, sigabrt, sigill, ... etc.

If so, shouldn't we add to the configurator?

Yes it would be nice. Could you send a PR?

[momvart]

Yes it would be nice. Could you send a PR?

Sure. Any recommendations for the API design? How about this?

pub trait CommandConfigurator<I, C = Child>: Sized {
    // ...
    fn exit_kind_from_status(&self, status: &std::process::ExitStatus) -> ExitKind {
        match status.signal() {
            Some(9) => ExitKind::Oom,
            Some(_) => ExitKind::Crash,
            None => ExitKind::Ok,
        }  
    }
}

[tokatoka]

How should it be used?

[domenukk]

I think we could have crash_exit_code(status) and oom_exit_code(status) functions in the builder Alternatively we could have an optional closure that runs after each execution and returns ExitKind

[momvart]

How should it be used?

If you mean the user side, they can provide a custom implementation for it as a part of their configurator. In CommandExecutor it will look like this:

-        let res = match child
+        let exit_kind = child
             .wait_timeout(self.configurer.exec_timeout())
             .expect("waiting on child failed")
-            .map(|status| status.signal())
-        {
-            // for reference: https://www.man7.org/linux/man-pages/man7/signal.7.html
-            Some(Some(9)) => Ok(ExitKind::Oom),
-            Some(Some(_)) => Ok(ExitKind::Crash),
-            Some(None) => Ok(ExitKind::Ok),
-            None => {
+            .map(|status| self.configurer.exit_kind_from_status(&status))
+            .unwrap_or_else(|| {
                 // if this fails, there is not much we can do. let's hope it failed because the process finished
                 // in the meantime.
                 drop(child.kill());
                 // finally, try to wait to properly clean up system resources.
                 drop(child.wait());
-                Ok(ExitKind::Timeout)
-            }
-        };
+                ExitKind::Timeout
+            });

-        if let Ok(exit_kind) = res {
-            self.observers
-                .post_exec_child_all(state, input, &exit_kind)?;
-        }
+        self.observers
+            .post_exec_child_all(state, input, &exit_kind)?;

[momvart]

an optional closure that runs after each execution and returns ExitKind

I think my suggestion is as same as this but as a function in CommandConfigurator.