Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 52 additions & 8 deletions crates/libafl/src/events/launcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ where
}
}

Self::wait_for_pids(&handles, self.spawn_broker);
Self::wait_for_pids(&handles, self.spawn_broker)?;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

delete ?;.

}
// This is the fork part for unix
#[cfg(not(unix))]
Expand Down Expand Up @@ -892,13 +892,16 @@ where
spawn_mgr(&self, None, monitor)?;
}

Self::wait_for_child_processes(&mut handles, self.spawn_broker);
Self::wait_for_child_processes(&mut handles, self.spawn_broker)?;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

delete ?;

}
Ok(())
Err(Error::shutting_down())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Delete this line

}

#[cfg(unix)]
fn wait_for_pids(handles: &[i32], spawn_broker: bool) {
fn wait_for_pids(handles: &[i32], spawn_broker: bool) -> Result<(), Error> {
let mut crash_count = 0;
let mut reasons = vec![];

if spawn_broker {
// Broker exited. kill all clients and wait for them to avoid zombies.
for handle in handles {
Expand Down Expand Up @@ -927,26 +930,67 @@ where
);
} else {
log::info!("Client with pid {handle} exited with status {status}");
crash_count += 1;
reasons.push(format!("Exited with status {status}"));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will get formatted weird by :?, maybe just push the status codes?

}
}
}
}

if crash_count > 0 {
return Err(Error::unknown(format!(
"{crash_count} child processes crashed. Reasons: {reasons:?}"
)));
}
Ok(())
}

fn wait_for_child_processes(handles: &mut [std::process::Child], spawn_broker: bool) {
fn wait_for_child_processes(
handles: &mut [std::process::Child],
spawn_broker: bool,
) -> Result<(), Error> {
let mut crash_count = 0;
let mut reasons = vec![];

if spawn_broker {
for handle in &mut *handles {
let _ = handle.kill();
let _ = handle.wait();
}
} else {
for handle in &mut *handles {
let ecode = handle.wait();
if ecode.as_ref().is_ok_and(|e| !e.success()) {
log::info!("Client with handle {handle:?} exited with {ecode:?}");
let ecode = handle.wait()?;
if !ecode.success() {
#[cfg(unix)]
{
use std::os::unix::process::ExitStatusExt;
if let Some(sig) = ecode.signal() {
if sig != libc::SIGINT && sig != libc::SIGTERM {
crash_count += 1;
reasons.push(format!("Killed by signal {sig}"));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Similarly here, might be better to make a function-specific derived-debug enum with Killed(c_int), Exited(c_int) and push them here.

}
} else if let Some(code) = ecode.code() {
crash_count += 1;
reasons.push(format!("Exited with code {code}"));
}
}
#[cfg(not(unix))]
{
if let Some(code) = ecode.code() {
crash_count += 1;
reasons.push(format!("Exited with code {code}"));
}
}
}
}
}

if crash_count > 0 {
return Err(Error::unknown(format!(
"{crash_count} child processes crashed. Reasons: {reasons:?}"
)));
}
Ok(())
}

/// Launch the common broker logic
Expand Down
59 changes: 33 additions & 26 deletions fuzzers/forkserver/libafl-fuzz/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion fuzzers/forkserver/libafl-fuzz/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ fn main() {
res
};
match res {
Ok(()) => unreachable!(),
Ok(()) => (),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No, this shouldn't be the case; forkserver should never return "Ok", this should indeed be unreachable.

Err(Error::ShuttingDown) => println!("Fuzzing stopped by user. Good bye."),
Err(err) => panic!("Failed to run launcher: {err:?}"),
}
Expand Down
Loading