Fix child reaping on fuzzer termination

This commit contains the following changes:
    - Call `waitpid()` on the child and the fork server when terminating the
      fuzzer; thus, we do not end up with zombies.
    - Rename `fsrv.kill_signal` to `fsrv.child_kill_signal`, since the
      documentation states that the signal is used to terminate the *child*.
    - Use SIGTERM instead of fsrv.(child)_kill_signal, thus the fork server
      can always reap the child.
This commit is contained in:
Nils Bars
2022-10-20 13:14:29 +02:00
parent 5ccf389414
commit f84ea69660
9 changed files with 26 additions and 28 deletions

View File

@ -163,7 +163,7 @@ typedef struct afl_forkserver {
void (*add_extra_func)(void *afl_ptr, u8 *mem, u32 len); void (*add_extra_func)(void *afl_ptr, u8 *mem, u32 len);
u8 kill_signal; u8 child_kill_signal;
u8 persistent_mode; u8 persistent_mode;
#ifdef __linux__ #ifdef __linux__
@ -222,4 +222,3 @@ void afl_fsrv_kill(afl_forkserver_t *fsrv);
#endif /* ^RLIMIT_AS */ #endif /* ^RLIMIT_AS */
#endif #endif

View File

@ -159,6 +159,7 @@ static void at_exit(int signal) {
if (unlikely(child_pid > 0)) { if (unlikely(child_pid > 0)) {
kill(child_pid, SIGKILL); kill(child_pid, SIGKILL);
waitpid(child_pid, NULL, 0);
child_pid = -1; child_pid = -1;
} }
@ -2407,4 +2408,3 @@ void __afl_set_persistent_mode(u8 mode) {
} }
#undef write_error #undef write_error

View File

@ -114,7 +114,7 @@ static void kill_child() {
if (fsrv.child_pid > 0) { if (fsrv.child_pid > 0) {
kill(fsrv.child_pid, fsrv.kill_signal); kill(fsrv.child_pid, fsrv.child_kill_signal);
fsrv.child_pid = -1; fsrv.child_pid = -1;
} }
@ -1115,7 +1115,7 @@ int main(int argc, char **argv_orig, char **envp) {
} }
fsrv.kill_signal = fsrv.child_kill_signal =
parse_afl_kill_signal_env(getenv("AFL_KILL_SIGNAL"), SIGKILL); parse_afl_kill_signal_env(getenv("AFL_KILL_SIGNAL"), SIGKILL);
read_initial_file(); read_initial_file();
@ -1151,4 +1151,3 @@ int main(int argc, char **argv_orig, char **envp) {
exit(0); exit(0);
} }

View File

@ -100,7 +100,7 @@ void afl_fsrv_init(afl_forkserver_t *fsrv) {
fsrv->init_tmout = EXEC_TIMEOUT * FORK_WAIT_MULT; fsrv->init_tmout = EXEC_TIMEOUT * FORK_WAIT_MULT;
fsrv->mem_limit = MEM_LIMIT; fsrv->mem_limit = MEM_LIMIT;
fsrv->out_file = NULL; fsrv->out_file = NULL;
fsrv->kill_signal = SIGKILL; fsrv->child_kill_signal = SIGKILL;
/* exec related stuff */ /* exec related stuff */
fsrv->child_pid = -1; fsrv->child_pid = -1;
@ -134,7 +134,7 @@ void afl_fsrv_init_dup(afl_forkserver_t *fsrv_to, afl_forkserver_t *from) {
fsrv_to->no_unlink = from->no_unlink; fsrv_to->no_unlink = from->no_unlink;
fsrv_to->uses_crash_exitcode = from->uses_crash_exitcode; fsrv_to->uses_crash_exitcode = from->uses_crash_exitcode;
fsrv_to->crash_exitcode = from->crash_exitcode; fsrv_to->crash_exitcode = from->crash_exitcode;
fsrv_to->kill_signal = from->kill_signal; fsrv_to->child_kill_signal = from->child_kill_signal;
fsrv_to->debug = from->debug; fsrv_to->debug = from->debug;
// These are forkserver specific. // These are forkserver specific.
@ -793,7 +793,7 @@ void afl_fsrv_start(afl_forkserver_t *fsrv, char **argv,
s32 tmp_pid = fsrv->fsrv_pid; s32 tmp_pid = fsrv->fsrv_pid;
if (tmp_pid > 0) { if (tmp_pid > 0) {
kill(tmp_pid, fsrv->kill_signal); kill(tmp_pid, fsrv->child_kill_signal);
fsrv->fsrv_pid = -1; fsrv->fsrv_pid = -1;
} }
@ -804,7 +804,7 @@ void afl_fsrv_start(afl_forkserver_t *fsrv, char **argv,
s32 tmp_pid = fsrv->fsrv_pid; s32 tmp_pid = fsrv->fsrv_pid;
if (tmp_pid > 0) { if (tmp_pid > 0) {
kill(tmp_pid, fsrv->kill_signal); kill(tmp_pid, fsrv->child_kill_signal);
fsrv->fsrv_pid = -1; fsrv->fsrv_pid = -1;
} }
@ -1242,10 +1242,10 @@ void afl_fsrv_start(afl_forkserver_t *fsrv, char **argv,
void afl_fsrv_kill(afl_forkserver_t *fsrv) { void afl_fsrv_kill(afl_forkserver_t *fsrv) {
if (fsrv->child_pid > 0) { kill(fsrv->child_pid, fsrv->kill_signal); } if (fsrv->child_pid > 0) { kill(fsrv->child_pid, fsrv->child_kill_signal); }
if (fsrv->fsrv_pid > 0) { if (fsrv->fsrv_pid > 0) {
kill(fsrv->fsrv_pid, fsrv->kill_signal); kill(fsrv->fsrv_pid, SIGTERM);
if (waitpid(fsrv->fsrv_pid, NULL, 0) <= 0) { WARNF("error waitpid\n"); } if (waitpid(fsrv->fsrv_pid, NULL, 0) <= 0) { WARNF("error waitpid\n"); }
} }
@ -1545,7 +1545,7 @@ afl_fsrv_run_target(afl_forkserver_t *fsrv, u32 timeout,
s32 tmp_pid = fsrv->child_pid; s32 tmp_pid = fsrv->child_pid;
if (tmp_pid > 0) { if (tmp_pid > 0) {
kill(tmp_pid, fsrv->kill_signal); kill(tmp_pid, fsrv->child_kill_signal);
fsrv->child_pid = -1; fsrv->child_pid = -1;
} }
@ -1605,7 +1605,7 @@ afl_fsrv_run_target(afl_forkserver_t *fsrv, u32 timeout,
/* Did we timeout? */ /* Did we timeout? */
if (unlikely(fsrv->last_run_timed_out)) { if (unlikely(fsrv->last_run_timed_out)) {
fsrv->last_kill_signal = fsrv->kill_signal; fsrv->last_kill_signal = fsrv->child_kill_signal;
return FSRV_RUN_TMOUT; return FSRV_RUN_TMOUT;
} }
@ -1688,4 +1688,3 @@ void afl_fsrv_deinit(afl_forkserver_t *fsrv) {
list_remove(&fsrv_list, fsrv); list_remove(&fsrv_list, fsrv);
} }

View File

@ -2963,4 +2963,3 @@ void save_cmdline(afl_state_t *afl, u32 argc, char **argv) {
*buf = 0; *buf = 0;
} }

View File

@ -23,6 +23,7 @@
*/ */
#include <signal.h>
#include "afl-fuzz.h" #include "afl-fuzz.h"
#include "envs.h" #include "envs.h"
@ -653,9 +654,14 @@ void afl_states_stop(void) {
}); });
LIST_FOREACH(&afl_states, afl_state_t, { LIST_FOREACH(&afl_states, afl_state_t, {
/* NOTE: We need to make sure that the parent (the forkserver) reap the child (see below). */
if (el->fsrv.child_pid > 0) kill(el->fsrv.child_pid, el->fsrv.kill_signal); if (el->fsrv.child_pid > 0) kill(el->fsrv.child_pid, el->fsrv.child_kill_signal);
if (el->fsrv.fsrv_pid > 0) kill(el->fsrv.fsrv_pid, el->fsrv.kill_signal); if (el->fsrv.fsrv_pid > 0) {
/* This must be SIGTERM, to allow the forkserver to reap the child before exiting. */
kill(el->fsrv.fsrv_pid, SIGTERM);
/* Make sure the forkserver does not end up as zombie. */
waitpid(el->fsrv.fsrv_pid, NULL, 0);
}
}); });
@ -672,4 +678,3 @@ void afl_states_request_skip(void) {
LIST_FOREACH(&afl_states, afl_state_t, { el->skip_requested = 1; }); LIST_FOREACH(&afl_states, afl_state_t, { el->skip_requested = 1; });
} }

View File

@ -1358,7 +1358,7 @@ int main(int argc, char **argv_orig, char **envp) {
#endif #endif
afl->fsrv.kill_signal = afl->fsrv.child_kill_signal =
parse_afl_kill_signal_env(afl->afl_env.afl_kill_signal, SIGKILL); parse_afl_kill_signal_env(afl->afl_env.afl_kill_signal, SIGKILL);
setup_signal_handlers(); setup_signal_handlers();
@ -2683,4 +2683,3 @@ stop_fuzzing:
} }
#endif /* !AFL_LIB */ #endif /* !AFL_LIB */

View File

@ -129,7 +129,7 @@ static void kill_child() {
timed_out = 1; timed_out = 1;
if (fsrv->child_pid > 0) { if (fsrv->child_pid > 0) {
kill(fsrv->child_pid, fsrv->kill_signal); kill(fsrv->child_pid, fsrv->child_kill_signal);
fsrv->child_pid = -1; fsrv->child_pid = -1;
} }
@ -1258,7 +1258,7 @@ int main(int argc, char **argv_orig, char **envp) {
: 0); : 0);
be_quiet = save_be_quiet; be_quiet = save_be_quiet;
fsrv->kill_signal = fsrv->child_kill_signal =
parse_afl_kill_signal_env(getenv("AFL_KILL_SIGNAL"), SIGKILL); parse_afl_kill_signal_env(getenv("AFL_KILL_SIGNAL"), SIGKILL);
if (new_map_size) { if (new_map_size) {
@ -1472,4 +1472,3 @@ int main(int argc, char **argv_orig, char **envp) {
exit(ret); exit(ret);
} }

View File

@ -113,7 +113,7 @@ static void kill_child() {
if (fsrv->child_pid > 0) { if (fsrv->child_pid > 0) {
kill(fsrv->child_pid, fsrv->kill_signal); kill(fsrv->child_pid, fsrv->child_kill_signal);
fsrv->child_pid = -1; fsrv->child_pid = -1;
} }
@ -1195,7 +1195,7 @@ int main(int argc, char **argv_orig, char **envp) {
} }
fsrv->kill_signal = fsrv->child_kill_signal =
parse_afl_kill_signal_env(getenv("AFL_KILL_SIGNAL"), SIGKILL); parse_afl_kill_signal_env(getenv("AFL_KILL_SIGNAL"), SIGKILL);
if (getenv("AFL_CRASH_EXITCODE")) { if (getenv("AFL_CRASH_EXITCODE")) {
@ -1351,4 +1351,3 @@ int main(int argc, char **argv_orig, char **envp) {
exit(0); exit(0);
} }