Merge pull request #1563 from nbars/stable

Fix child reaping on fuzzer termination

8 files changed, 76 insertions, 55 deletions

diff --git a/src/afl-analyze.c b/src/afl-analyze.c
index a21f014f..d356874d 100644
--- a/src/afl-analyze.c
+++ b/src/afl-analyze.c
@@ -114,7 +114,7 @@ static void kill_child() {
 
   if (fsrv.child_pid > 0) {
 
-    kill(fsrv.child_pid, fsrv.kill_signal);
+    kill(fsrv.child_pid, fsrv.child_kill_signal);
     fsrv.child_pid = -1;
 
   }
@@ -1115,8 +1115,8 @@ int main(int argc, char **argv_orig, char **envp) {
 
   }
 
-  fsrv.kill_signal =
-      parse_afl_kill_signal_env(getenv("AFL_KILL_SIGNAL"), SIGKILL);
+  configure_afl_kill_signals(&fsrv, NULL, NULL);
+
 
   read_initial_file();
   (void)check_binary_signatures(fsrv.target_path);
@@ -1151,4 +1151,3 @@ int main(int argc, char **argv_orig, char **envp) {
   exit(0);
 
 }
-
diff --git a/src/afl-common.c b/src/afl-common.c
index f3e78ac5..f2934817 100644
--- a/src/afl-common.c
+++ b/src/afl-common.c
@@ -25,6 +25,7 @@
 
 #include <stdlib.h>
 #include <stdio.h>
+#include "forkserver.h"
 #ifndef _GNU_SOURCE
   #define _GNU_SOURCE
 #endif
@@ -47,6 +48,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <signal.h>
 
 u8  be_quiet = 0;
 u8 *doc_path = "";
@@ -456,37 +458,44 @@ u8 *find_afl_binary(u8 *own_loc, u8 *fname) {
 
 }
 
-/* Parses the kill signal environment variable, FATALs on error.
-  If the env is not set, sets the env to default_signal for the signal handlers
-  and returns the default_signal. */
-int parse_afl_kill_signal_env(u8 *afl_kill_signal_env, int default_signal) {
 
-  if (afl_kill_signal_env && afl_kill_signal_env[0]) {
+int parse_afl_kill_signal(u8 *numeric_signal_as_str, int default_signal) {
+
+  if (numeric_signal_as_str && numeric_signal_as_str[0]) {
 
     char *endptr;
     u8    signal_code;
-    signal_code = (u8)strtoul(afl_kill_signal_env, &endptr, 10);
+    signal_code = (u8)strtoul(numeric_signal_as_str, &endptr, 10);
     /* Did we manage to parse the full string? */
-    if (*endptr != '\0' || endptr == (char *)afl_kill_signal_env) {
-
-      FATAL("Invalid AFL_KILL_SIGNAL: %s (expected unsigned int)",
-            afl_kill_signal_env);
-
+    if (*endptr != '\0' || endptr == (char *)numeric_signal_as_str) {
+      FATAL("Invalid signal name: %s", numeric_signal_as_str);
+    } else {
+      return signal_code;
     }
 
-    return signal_code;
-
-  } else {
-
-    char *sigstr = alloc_printf("%d", default_signal);
-    if (!sigstr) { FATAL("Failed to alloc mem for signal buf"); }
+  }
 
-    /* Set the env for signal handler */
-    setenv("AFL_KILL_SIGNAL", sigstr, 1);
-    free(sigstr);
-    return default_signal;
+  return default_signal;
+}
 
+void configure_afl_kill_signals(afl_forkserver_t *fsrv, char* afl_kill_signal_env, char* afl_fsrv_kill_signal_env) {
+  afl_kill_signal_env = afl_kill_signal_env ?
+    afl_kill_signal_env : getenv("AFL_KILL_SIGNAL");
+  afl_fsrv_kill_signal_env = afl_fsrv_kill_signal_env ?
+    afl_fsrv_kill_signal_env : getenv("AFL_FORK_SERVER_KILL_SIGNAL");
+
+  fsrv->child_kill_signal =
+      parse_afl_kill_signal(afl_kill_signal_env, SIGKILL);
+
+  if (afl_kill_signal_env && !afl_fsrv_kill_signal_env) {
+    /*
+    Set AFL_FORK_SERVER_KILL_SIGNAL to the value of AFL_KILL_SIGNAL for backwards
+    compatibility. However, if AFL_FORK_SERVER_KILL_SIGNAL is set, is takes precedence.
+    */
+    afl_fsrv_kill_signal_env = afl_kill_signal_env;
   }
+  fsrv->fsrv_kill_signal =
+      parse_afl_kill_signal(afl_fsrv_kill_signal_env, SIGTERM);
 
 }
 
@@ -1253,4 +1262,3 @@ s32 create_file(u8 *fn) {
   return fd;
 
 }
-
diff --git a/src/afl-forkserver.c b/src/afl-forkserver.c
index 628ff590..72db3c2e 100644
--- a/src/afl-forkserver.c
+++ b/src/afl-forkserver.c
@@ -100,7 +100,7 @@ void afl_fsrv_init(afl_forkserver_t *fsrv) {
   fsrv->init_tmout = EXEC_TIMEOUT * FORK_WAIT_MULT;
   fsrv->mem_limit = MEM_LIMIT;
   fsrv->out_file = NULL;
-  fsrv->kill_signal = SIGKILL;
+  fsrv->child_kill_signal = SIGKILL;
 
   /* exec related stuff */
   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->uses_crash_exitcode = from->uses_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;
 
   // These are forkserver specific.
@@ -793,7 +793,7 @@ void afl_fsrv_start(afl_forkserver_t *fsrv, char **argv,
       s32 tmp_pid = fsrv->fsrv_pid;
       if (tmp_pid > 0) {
 
-        kill(tmp_pid, fsrv->kill_signal);
+        kill(tmp_pid, fsrv->child_kill_signal);
         fsrv->fsrv_pid = -1;
 
       }
@@ -804,7 +804,7 @@ void afl_fsrv_start(afl_forkserver_t *fsrv, char **argv,
       s32 tmp_pid = fsrv->fsrv_pid;
       if (tmp_pid > 0) {
 
-        kill(tmp_pid, fsrv->kill_signal);
+        kill(tmp_pid, fsrv->child_kill_signal);
         fsrv->fsrv_pid = -1;
 
       }
@@ -1242,11 +1242,11 @@ void afl_fsrv_start(afl_forkserver_t *fsrv, char **argv,
 
 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) {
 
-    kill(fsrv->fsrv_pid, fsrv->kill_signal);
-    if (waitpid(fsrv->fsrv_pid, NULL, 0) <= 0) { WARNF("error waitpid\n"); }
+    kill(fsrv->fsrv_pid, fsrv->fsrv_kill_signal);
+    waitpid(fsrv->fsrv_pid, NULL, 0);
 
   }
 
@@ -1545,7 +1545,7 @@ afl_fsrv_run_target(afl_forkserver_t *fsrv, u32 timeout,
     s32 tmp_pid = fsrv->child_pid;
     if (tmp_pid > 0) {
 
-      kill(tmp_pid, fsrv->kill_signal);
+      kill(tmp_pid, fsrv->child_kill_signal);
       fsrv->child_pid = -1;
 
     }
@@ -1605,7 +1605,7 @@ afl_fsrv_run_target(afl_forkserver_t *fsrv, u32 timeout,
   /* Did we timeout? */
   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;
 
   }
@@ -1688,4 +1688,3 @@ void afl_fsrv_deinit(afl_forkserver_t *fsrv) {
   list_remove(&fsrv_list, fsrv);
 
 }
-
diff --git a/src/afl-fuzz-init.c b/src/afl-fuzz-init.c
index e41d29fd..fded44ac 100644
--- a/src/afl-fuzz-init.c
+++ b/src/afl-fuzz-init.c
@@ -2963,4 +2963,3 @@ void save_cmdline(afl_state_t *afl, u32 argc, char **argv) {
   *buf = 0;
 
 }
-
diff --git a/src/afl-fuzz-state.c b/src/afl-fuzz-state.c
index 5199f7e6..ae6cb6c7 100644
--- a/src/afl-fuzz-state.c
+++ b/src/afl-fuzz-state.c
@@ -23,6 +23,7 @@
 
  */
 
+#include <signal.h>
 #include "afl-fuzz.h"
 #include "envs.h"
 
@@ -484,10 +485,15 @@ void read_afl_environment(afl_state_t *afl, char **envp) {
 #endif
 
           } else if (!strncmp(env, "AFL_KILL_SIGNAL",
+                              afl_environment_variable_len)) {
+
+            afl->afl_env.afl_child_kill_signal =
+                (u8 *)get_afl_env(afl_environment_variables[i]);
 
+          } else if (!strncmp(env, "AFL_FORK_SERVER_KILL_SIGNAL",
                               afl_environment_variable_len)) {
 
-            afl->afl_env.afl_kill_signal =
+            afl->afl_env.afl_fsrv_kill_signal =
                 (u8 *)get_afl_env(afl_environment_variables[i]);
 
           } else if (!strncmp(env, "AFL_TARGET_ENV",
@@ -653,9 +659,13 @@ void afl_states_stop(void) {
   });
 
   LIST_FOREACH(&afl_states, afl_state_t, {
-
-    if (el->fsrv.child_pid > 0) kill(el->fsrv.child_pid, el->fsrv.kill_signal);
-    if (el->fsrv.fsrv_pid > 0) kill(el->fsrv.fsrv_pid, el->fsrv.kill_signal);
+    /* 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.child_kill_signal);
+    if (el->fsrv.fsrv_pid > 0) {
+      kill(el->fsrv.fsrv_pid, el->fsrv.fsrv_kill_signal);
+      /* Make sure the forkserver does not end up as zombie. */
+      waitpid(el->fsrv.fsrv_pid, NULL, 0);
+    }
 
   });
 
@@ -672,4 +682,3 @@ void afl_states_request_skip(void) {
   LIST_FOREACH(&afl_states, afl_state_t, { el->skip_requested = 1; });
 
 }
-
diff --git a/src/afl-fuzz.c b/src/afl-fuzz.c
index d116822a..d8d804ae 100644
--- a/src/afl-fuzz.c
+++ b/src/afl-fuzz.c
@@ -25,6 +25,7 @@
 
 #include "afl-fuzz.h"
 #include "cmplog.h"
+#include "common.h"
 #include <limits.h>
 #include <stdlib.h>
 #ifndef USEMMAP
@@ -261,6 +262,9 @@ static void usage(u8 *argv0, int more_help) {
       "AFL_INPUT_LEN_MIN/AFL_INPUT_LEN_MAX: like -g/-G set min/max fuzz length produced\n"
       "AFL_PIZZA_MODE: 1 - enforce pizza mode, 0 - disable for April 1st\n"
       "AFL_KILL_SIGNAL: Signal ID delivered to child processes on timeout, etc. (default: SIGKILL)\n"
+      "AFL_FORK_SERVER_KILL_SIGNAL: Signal delivered to fork server processes on termination\n"
+      "                             (default: SIGTERM). If this is not set and AFL_KILL_SIGNAL is set,\n"
+      "                             this will be set to the same value.\n"
       "AFL_MAP_SIZE: the shared memory size for that target. must be >= the size\n"
       "              the target was compiled for\n"
       "AFL_MAX_DET_EXTRAS: if more entries are in the dictionary list than this value\n"
@@ -1358,8 +1362,9 @@ int main(int argc, char **argv_orig, char **envp) {
 
   #endif
 
-  afl->fsrv.kill_signal =
-      parse_afl_kill_signal_env(afl->afl_env.afl_kill_signal, SIGKILL);
+  configure_afl_kill_signals(&afl->fsrv,
+    afl->afl_env.afl_child_kill_signal,
+    afl->afl_env.afl_fsrv_kill_signal);
 
   setup_signal_handlers();
   check_asan_opts(afl);
@@ -2683,4 +2688,3 @@ stop_fuzzing:
 }
 
 #endif                                                          /* !AFL_LIB */
-
diff --git a/src/afl-showmap.c b/src/afl-showmap.c
index 5e3fb67d..31091e8e 100644
--- a/src/afl-showmap.c
+++ b/src/afl-showmap.c
@@ -129,7 +129,7 @@ static void kill_child() {
   timed_out = 1;
   if (fsrv->child_pid > 0) {
 
-    kill(fsrv->child_pid, fsrv->kill_signal);
+    kill(fsrv->child_pid, fsrv->child_kill_signal);
     fsrv->child_pid = -1;
 
   }
@@ -864,8 +864,11 @@ static void usage(u8 *argv0) {
       "AFL_DEBUG: enable extra developer output\n"
       "AFL_FORKSRV_INIT_TMOUT: time spent waiting for forkserver during "
       "startup (in milliseconds)\n"
-      "AFL_KILL_SIGNAL: Signal ID delivered to child processes on timeout, "
-      "etc. (default: SIGKILL)\n"
+      "AFL_KILL_SIGNAL: Signal ID delivered to child processes on timeout,\n"
+      "                 etc. (default: SIGKILL)\n"
+      "AFL_FORK_SERVER_KILL_SIGNAL: Signal delivered to fork server processes on termination\n"
+      "                             (default: SIGTERM). If this is not set and AFL_KILL_SIGNAL is set,\n"
+      "                             this will be set to the same value as AFL_KILL_SIGNAL.\n"
       "AFL_MAP_SIZE: the shared memory size for that target. must be >= the "
       "size the target was compiled for\n"
       "AFL_PRELOAD: LD_PRELOAD / DYLD_INSERT_LIBRARIES settings for target\n"
@@ -1258,8 +1261,7 @@ int main(int argc, char **argv_orig, char **envp) {
                                  : 0);
     be_quiet = save_be_quiet;
 
-    fsrv->kill_signal =
-        parse_afl_kill_signal_env(getenv("AFL_KILL_SIGNAL"), SIGKILL);
+    configure_afl_kill_signals(fsrv, NULL, NULL);
 
     if (new_map_size) {
 
@@ -1472,4 +1474,3 @@ int main(int argc, char **argv_orig, char **envp) {
   exit(ret);
 
 }
-
diff --git a/src/afl-tmin.c b/src/afl-tmin.c
index 694c9c21..b346f65c 100644
--- a/src/afl-tmin.c
+++ b/src/afl-tmin.c
@@ -113,7 +113,7 @@ static void kill_child() {
 
   if (fsrv->child_pid > 0) {
 
-    kill(fsrv->child_pid, fsrv->kill_signal);
+    kill(fsrv->child_pid, fsrv->child_kill_signal);
     fsrv->child_pid = -1;
 
   }
@@ -881,6 +881,9 @@ static void usage(u8 *argv0) {
       "AFL_CRASH_EXITCODE: optional child exit code to be interpreted as crash\n"
       "AFL_FORKSRV_INIT_TMOUT: time spent waiting for forkserver during startup (in milliseconds)\n"
       "AFL_KILL_SIGNAL: Signal ID delivered to child processes on timeout, etc. (default: SIGKILL)\n"
+      "AFL_FORK_SERVER_KILL_SIGNAL: Signal delivered to fork server processes on termination\n"
+      "                             (default: SIGTERM). If this is not set and AFL_KILL_SIGNAL is set,\n"
+      "                             this will be set to the same value as AFL_KILL_SIGNAL.\n"
       "AFL_MAP_SIZE: the shared memory size for that target. must be >= the size\n"
       "              the target was compiled for\n"
       "AFL_PRELOAD:  LD_PRELOAD / DYLD_INSERT_LIBRARIES settings for target\n"
@@ -1195,8 +1198,8 @@ int main(int argc, char **argv_orig, char **envp) {
 
   }
 
-  fsrv->kill_signal =
-      parse_afl_kill_signal_env(getenv("AFL_KILL_SIGNAL"), SIGKILL);
+  configure_afl_kill_signals(fsrv, NULL, NULL);
+
 
   if (getenv("AFL_CRASH_EXITCODE")) {
 
@@ -1351,4 +1354,3 @@ int main(int argc, char **argv_orig, char **envp) {
   exit(0);
 
 }
-