1 files changed, 190 insertions, 82 deletions

diff --git a/src/common/util.c b/src/common/util.c
index e7059a5bd9..22c816abcc 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -2157,13 +2157,12 @@ write_chunks_to_file(const char *fname, const smartlist_t *chunks, int bin)
   return write_chunks_to_file_impl(fname, chunks, flags);
 }
 
-/** As write_str_to_file, but does not assume a NUL-terminated
- * string. Instead, we write <b>len</b> bytes, starting at <b>str</b>. */
-int
-write_bytes_to_file(const char *fname, const char *str, size_t len,
-                    int bin)
+/** Write <b>len</b> bytes, starting at <b>str</b>, to <b>fname</b>
+    using the open() flags passed in <b>flags</b>. */
+static int
+write_bytes_to_file_impl(const char *fname, const char *str, size_t len,
+                         int flags)
 {
-  int flags = OPEN_FLAGS_REPLACE|(bin?O_BINARY:O_TEXT);
   int r;
   sized_chunk_t c = { str, len };
   smartlist_t *chunks = smartlist_create();
@@ -2173,20 +2172,35 @@ write_bytes_to_file(const char *fname, const char *str, size_t len,
   return r;
 }
 
+/** As write_str_to_file, but does not assume a NUL-terminated
+ * string. Instead, we write <b>len</b> bytes, starting at <b>str</b>. */
+int
+write_bytes_to_file(const char *fname, const char *str, size_t len,
+                    int bin)
+{
+  return write_bytes_to_file_impl(fname, str, len,
+                                  OPEN_FLAGS_REPLACE|(bin?O_BINARY:O_TEXT));
+}
+
 /** As write_bytes_to_file, but if the file already exists, append the bytes
  * to the end of the file instead of overwriting it. */
 int
 append_bytes_to_file(const char *fname, const char *str, size_t len,
                      int bin)
 {
-  int flags = OPEN_FLAGS_APPEND|(bin?O_BINARY:O_TEXT);
-  int r;
-  sized_chunk_t c = { str, len };
-  smartlist_t *chunks = smartlist_create();
-  smartlist_add(chunks, &c);
-  r = write_chunks_to_file_impl(fname, chunks, flags);
-  smartlist_free(chunks);
-  return r;
+  return write_bytes_to_file_impl(fname, str, len,
+                                  OPEN_FLAGS_APPEND|(bin?O_BINARY:O_TEXT));
+}
+
+/** Like write_str_to_file(), but also return -1 if there was a file
+    already residing in <b>fname</b>. */
+int
+write_bytes_to_new_file(const char *fname, const char *str, size_t len,
+                        int bin)
+{
+  return write_bytes_to_file_impl(fname, str, len,
+                                  OPEN_FLAGS_DONT_REPLACE|
+                                  (bin?O_BINARY:O_TEXT));
 }
 
 /** Read the contents of <b>filename</b> into a newly allocated
@@ -3179,28 +3193,72 @@ format_helper_exit_status(unsigned char child_state, int saved_errno,
 /* Maximum number of file descriptors, if we cannot get it via sysconf() */
 #define DEFAULT_MAX_FD 256
 
-/** Terminate process running at PID <b>pid</b>.
+/** Terminate the process of <b>process_handle</b>.
  *  Code borrowed from Python's os.kill. */
 int
-tor_terminate_process(pid_t pid)
+tor_terminate_process(process_handle_t *process_handle)
 {
 #ifdef MS_WINDOWS
-  HANDLE handle;
-  /* If the signal is outside of what GenerateConsoleCtrlEvent can use,
-     attempt to open and terminate the process. */
-  handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid);
-  if (!handle)
-    return -1;
+  if (tor_get_exit_code(process_handle, 0, NULL) == PROCESS_EXIT_RUNNING) {
+    HANDLE handle;
+    /* If the signal is outside of what GenerateConsoleCtrlEvent can use,
+       attempt to open and terminate the process. */
+    handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE,
+                         process_handle->pid.dwProcessId);
+    if (!handle)
+      return -1;
 
-  if (!TerminateProcess(handle, 0))
-    return -1;
-  else
-    return 0;
+    if (!TerminateProcess(handle, 0))
+      return -1;
+    else
+      return 0;
+  }
 #else /* Unix */
-  return kill(pid, SIGTERM);
+  return kill(process_handle->pid, SIGTERM);
+#endif
+
+  return -1;
+}
+
+/** Return the Process ID of <b>process_handle</b>. */
+int
+tor_process_get_pid(process_handle_t *process_handle)
+{
+#ifdef MS_WINDOWS
+  return (int) process_handle->pid.dwProcessId;
+#else
+  return (int) process_handle->pid;
+#endif
+}
+
+#ifdef MS_WINDOWS
+HANDLE
+tor_process_get_stdout_pipe(process_handle_t *process_handle)
+{
+  return process_handle->stdout_pipe;
+}
+#else
+FILE *
+tor_process_get_stdout_pipe(process_handle_t *process_handle)
+{
+  return process_handle->stdout_handle;
+}
+#endif
+
+static process_handle_t *
+process_handle_new(void)
+{
+  process_handle_t *out = tor_malloc_zero(sizeof(process_handle_t));
+
+#ifndef MS_WINDOWS
+  out->stdout_pipe = -1;
+  out->stderr_pipe = -1;
 #endif
+
+  return out;
 }
 
+/*DOCDOC*/
 #define CHILD_STATE_INIT 0
 #define CHILD_STATE_PIPE 1
 #define CHILD_STATE_MAXFD 2
@@ -3233,14 +3291,20 @@ tor_terminate_process(pid_t pid)
  */
 int
 tor_spawn_background(const char *const filename, const char **argv,
+#ifdef MS_WINDOWS
+                     LPVOID envp,
+#else
                      const char **envp,
-                     process_handle_t *process_handle)
+#endif
+                     process_handle_t **process_handle_out)
 {
 #ifdef MS_WINDOWS
   HANDLE stdout_pipe_read = NULL;
   HANDLE stdout_pipe_write = NULL;
   HANDLE stderr_pipe_read = NULL;
   HANDLE stderr_pipe_write = NULL;
+  process_handle_t *process_handle;
+  int status;
 
   STARTUPINFO siStartInfo;
   BOOL retval = FALSE;
@@ -3250,30 +3314,26 @@ tor_spawn_background(const char *const filename, const char **argv,
 
   (void)envp; // Unused on Windows
 
-  /* process_handle must not be NULL */
-  tor_assert(process_handle != NULL);
-
   saAttr.nLength = sizeof(SECURITY_ATTRIBUTES);
   saAttr.bInheritHandle = TRUE;
   /* TODO: should we set explicit security attributes? (#2046, comment 5) */
   saAttr.lpSecurityDescriptor = NULL;
 
   /* Assume failure to start process */
-  memset(process_handle, 0, sizeof(process_handle_t));
-  process_handle->status = PROCESS_STATUS_ERROR;
+  status = PROCESS_STATUS_ERROR;
 
   /* Set up pipe for stdout */
   if (!CreatePipe(&stdout_pipe_read, &stdout_pipe_write, &saAttr, 0)) {
     log_warn(LD_GENERAL,
       "Failed to create pipe for stdout communication with child process: %s",
       format_win32_error(GetLastError()));
-    return process_handle->status;
+    return status;
   }
   if (!SetHandleInformation(stdout_pipe_read, HANDLE_FLAG_INHERIT, 0)) {
     log_warn(LD_GENERAL,
       "Failed to configure pipe for stdout communication with child "
       "process: %s", format_win32_error(GetLastError()));
-    return process_handle->status;
+    return status;
   }
 
   /* Set up pipe for stderr */
@@ -3281,13 +3341,13 @@ tor_spawn_background(const char *const filename, const char **argv,
     log_warn(LD_GENERAL,
       "Failed to create pipe for stderr communication with child process: %s",
       format_win32_error(GetLastError()));
-    return process_handle->status;
+    return status;
   }
   if (!SetHandleInformation(stderr_pipe_read, HANDLE_FLAG_INHERIT, 0)) {
     log_warn(LD_GENERAL,
       "Failed to configure pipe for stderr communication with child "
       "process: %s", format_win32_error(GetLastError()));
-    return process_handle->status;
+    return status;
   }
 
   /* Create the child process */
@@ -3296,6 +3356,9 @@ tor_spawn_background(const char *const filename, const char **argv,
    */
   joined_argv = tor_join_win_cmdline(argv);
 
+  process_handle = process_handle_new();
+  process_handle->status = status;
+
   ZeroMemory(&(process_handle->pid), sizeof(PROCESS_INFORMATION));
   ZeroMemory(&siStartInfo, sizeof(STARTUPINFO));
   siStartInfo.cb = sizeof(STARTUPINFO);
@@ -3315,7 +3378,7 @@ tor_spawn_background(const char *const filename, const char **argv,
   /*(TODO: set CREATE_NEW CONSOLE/PROCESS_GROUP to make GetExitCodeProcess()
    * work?) */
                  0,             // creation flags
-                 NULL,          // use parent's environment
+                 envp,          // use parent's environment
                  NULL,          // use parent's current directory
                  &siStartInfo,  // STARTUPINFO pointer
                  &(process_handle->pid));  // receives PROCESS_INFORMATION
@@ -3326,22 +3389,25 @@ tor_spawn_background(const char *const filename, const char **argv,
     log_warn(LD_GENERAL,
       "Failed to create child process %s: %s", filename?filename:argv[0],
       format_win32_error(GetLastError()));
+    tor_free(process_handle);
   } else  {
     /* TODO: Close hProcess and hThread in process_handle->pid? */
     process_handle->stdout_pipe = stdout_pipe_read;
     process_handle->stderr_pipe = stderr_pipe_read;
-    process_handle->status = PROCESS_STATUS_RUNNING;
+    status = process_handle->status = PROCESS_STATUS_RUNNING;
   }
 
   /* TODO: Close pipes on exit */
-
-  return process_handle->status;
+  *process_handle_out = process_handle;
+  return status;
 #else // MS_WINDOWS
   pid_t pid;
   int stdout_pipe[2];
   int stderr_pipe[2];
   int fd, retval;
   ssize_t nbytes;
+  process_handle_t *process_handle;
+  int status;
 
   const char *error_message = SPAWN_ERROR_MESSAGE;
   size_t error_message_length;
@@ -3354,9 +3420,7 @@ tor_spawn_background(const char *const filename, const char **argv,
 
   static int max_fd = -1;
 
-  /* Assume failure to start */
-  memset(process_handle, 0, sizeof(process_handle_t));
-  process_handle->status = PROCESS_STATUS_ERROR;
+  status = PROCESS_STATUS_ERROR;
 
   /* We do the strlen here because strlen() is not signal handler safe,
      and we are not allowed to use unsafe functions between fork and exec */
@@ -3370,7 +3434,7 @@ tor_spawn_background(const char *const filename, const char **argv,
     log_warn(LD_GENERAL,
       "Failed to set up pipe for stdout communication with child process: %s",
        strerror(errno));
-    return process_handle->status;
+    return status;
   }
 
   retval = pipe(stderr_pipe);
@@ -3378,7 +3442,7 @@ tor_spawn_background(const char *const filename, const char **argv,
     log_warn(LD_GENERAL,
       "Failed to set up pipe for stderr communication with child process: %s",
       strerror(errno));
-    return process_handle->status;
+    return status;
   }
 
   child_state = CHILD_STATE_MAXFD;
@@ -3468,7 +3532,7 @@ tor_spawn_background(const char *const filename, const char **argv,
 
     _exit(255);
     /* Never reached, but avoids compiler warning */
-    return process_handle->status;
+    return status;
   }
 
   /* In parent */
@@ -3479,9 +3543,11 @@ tor_spawn_background(const char *const filename, const char **argv,
     close(stdout_pipe[1]);
     close(stderr_pipe[0]);
     close(stderr_pipe[1]);
-    return process_handle->status;
+    return status;
   }
 
+  process_handle = process_handle_new();
+  process_handle->status = status;
   process_handle->pid = pid;
 
   /* TODO: If the child process forked but failed to exec, waitpid it */
@@ -3505,7 +3571,7 @@ tor_spawn_background(const char *const filename, const char **argv,
             strerror(errno));
   }
 
-  process_handle->status = PROCESS_STATUS_RUNNING;
+  status = process_handle->status = PROCESS_STATUS_RUNNING;
   /* Set stdout/stderr pipes to be non-blocking */
   fcntl(process_handle->stdout_pipe, F_SETFL, O_NONBLOCK);
   fcntl(process_handle->stderr_pipe, F_SETFL, O_NONBLOCK);
@@ -3513,10 +3579,52 @@ tor_spawn_background(const char *const filename, const char **argv,
   process_handle->stdout_handle = fdopen(process_handle->stdout_pipe, "r");
   process_handle->stderr_handle = fdopen(process_handle->stderr_pipe, "r");
 
+  *process_handle_out = process_handle;
   return process_handle->status;
 #endif // MS_WINDOWS
 }
 
+/** Destroy all resources allocated by the process handle in
+ *  <b>process_handle</b>.
+ *  If <b>also_terminate_process</b> is true, also terminate the
+ *  process of the process handle. */
+void
+tor_process_handle_destroy(process_handle_t *process_handle,
+                           int also_terminate_process)
+{
+  if (!process_handle)
+    return;
+
+  if (also_terminate_process) {
+    if (tor_terminate_process(process_handle) < 0) {
+      log_notice(LD_GENERAL, "Failed to terminate process with PID '%d'",
+                 tor_process_get_pid(process_handle));
+    } else {
+      log_info(LD_GENERAL, "Terminated process with PID '%d'",
+               tor_process_get_pid(process_handle));
+    }
+  }
+
+  process_handle->status = PROCESS_STATUS_NOTRUNNING;
+
+#ifdef MS_WINDOWS
+  if (process_handle->stdout_pipe)
+    CloseHandle(process_handle->stdout_pipe);
+
+  if (process_handle->stderr_pipe)
+    CloseHandle(process_handle->stderr_pipe);
+#else
+  if (process_handle->stdout_handle)
+    fclose(process_handle->stdout_handle);
+
+  if (process_handle->stderr_handle)
+    fclose(process_handle->stderr_handle);
+#endif
+
+  memset(process_handle, 0x0f, sizeof(process_handle_t));
+  tor_free(process_handle);
+}
+
 /** Get the exit code of a process specified by <b>process_handle</b> and store
  * it in <b>exit_code</b>, if set to a non-NULL value.  If <b>block</b> is set
  * to true, the call will block until the process has exited.  Otherwise if
@@ -3528,7 +3636,7 @@ tor_spawn_background(const char *const filename, const char **argv,
  * probably not work in Tor, because waitpid() is called in main.c to reap any
  * terminated child processes.*/
 int
-tor_get_exit_code(const process_handle_t process_handle,
+tor_get_exit_code(const process_handle_t *process_handle,
                   int block, int *exit_code)
 {
 #ifdef MS_WINDOWS
@@ -3537,14 +3645,14 @@ tor_get_exit_code(const process_handle_t process_handle,
 
   if (block) {
     /* Wait for the process to exit */
-    retval = WaitForSingleObject(process_handle.pid.hProcess, INFINITE);
+    retval = WaitForSingleObject(process_handle->pid.hProcess, INFINITE);
     if (retval != WAIT_OBJECT_0) {
       log_warn(LD_GENERAL, "WaitForSingleObject() failed (%d): %s",
               (int)retval, format_win32_error(GetLastError()));
       return PROCESS_EXIT_ERROR;
     }
   } else {
-    retval = WaitForSingleObject(process_handle.pid.hProcess, 0);
+    retval = WaitForSingleObject(process_handle->pid.hProcess, 0);
     if (WAIT_TIMEOUT == retval) {
       /* Process has not exited */
       return PROCESS_EXIT_RUNNING;
@@ -3556,7 +3664,7 @@ tor_get_exit_code(const process_handle_t process_handle,
   }
 
   if (exit_code != NULL) {
-    success = GetExitCodeProcess(process_handle.pid.hProcess,
+    success = GetExitCodeProcess(process_handle->pid.hProcess,
                                  (PDWORD)exit_code);
     if (!success) {
       log_warn(LD_GENERAL, "GetExitCodeProcess() failed: %s",
@@ -3568,19 +3676,19 @@ tor_get_exit_code(const process_handle_t process_handle,
   int stat_loc;
   int retval;
 
-  retval = waitpid(process_handle.pid, &stat_loc, block?0:WNOHANG);
+  retval = waitpid(process_handle->pid, &stat_loc, block?0:WNOHANG);
   if (!block && 0 == retval) {
     /* Process has not exited */
     return PROCESS_EXIT_RUNNING;
-  } else if (retval != process_handle.pid) {
-    log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s", process_handle.pid,
-             strerror(errno));
+  } else if (retval != process_handle->pid) {
+    log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s",
+             process_handle->pid, strerror(errno));
     return PROCESS_EXIT_ERROR;
   }
 
   if (!WIFEXITED(stat_loc)) {
     log_warn(LD_GENERAL, "Process %d did not exit normally",
-             process_handle.pid);
+             process_handle->pid);
     return PROCESS_EXIT_ERROR;
   }
 
@@ -3682,7 +3790,6 @@ tor_read_all_handle(FILE *h, char *buf, size_t count,
     if (NULL == retval) {
       if (feof(h)) {
         log_debug(LD_GENERAL, "fgets() reached end of file");
-        fclose(h);
         if (eof)
           *eof = 1;
         break;
@@ -3695,7 +3802,6 @@ tor_read_all_handle(FILE *h, char *buf, size_t count,
         } else {
           log_warn(LD_GENERAL, "fgets() from handle failed: %s",
                    strerror(errno));
-          fclose(h);
           return -1;
         }
       }
@@ -3855,12 +3961,10 @@ log_from_pipe(FILE *stream, int severity, const char *executable,
     r = get_string_from_pipe(stream, buf, sizeof(buf) - 1);
 
     if (r == IO_STREAM_CLOSED) {
-      fclose(stream);
       return 1;
     } else if (r == IO_STREAM_EAGAIN) {
       return 0;
     } else if (r == IO_STREAM_TERM) {
-      fclose(stream);
       return -1;
     }
 
@@ -3968,7 +4072,7 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
 
   /* Static variables are initialized to zero, so child_handle.status=0
    * which corresponds to it not running on startup */
-  static process_handle_t child_handle;
+  static process_handle_t *child_handle=NULL;
 
   static time_t time_to_run_helper = 0;
   int stdout_status, stderr_status, retval;
@@ -3994,46 +4098,50 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
   argv[9] = NULL;
 
   /* Start the child, if it is not already running */
-  if (child_handle.status != PROCESS_STATUS_RUNNING &&
+  if ((!child_handle || child_handle->status != PROCESS_STATUS_RUNNING) &&
       time_to_run_helper < now) {
+    int status;
+
     /* Assume tor-fw-helper will succeed, start it later*/
     time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_SUCCESS;
 
+    if (child_handle) {
+      tor_process_handle_destroy(child_handle, 1);
+      child_handle = NULL;
+    }
+
 #ifdef MS_WINDOWS
     /* Passing NULL as lpApplicationName makes Windows search for the .exe */
-    tor_spawn_background(NULL, argv, NULL, &child_handle);
+    status = tor_spawn_background(NULL, argv, NULL, &child_handle);
 #else
-    tor_spawn_background(filename, argv, NULL, &child_handle);
+    status = tor_spawn_background(filename, argv, NULL, &child_handle);
 #endif
-    if (PROCESS_STATUS_ERROR == child_handle.status) {
+
+    if (PROCESS_STATUS_ERROR == status) {
       log_warn(LD_GENERAL, "Failed to start port forwarding helper %s",
               filename);
       time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_FAIL;
       return;
     }
-#ifdef MS_WINDOWS
-    log_info(LD_GENERAL,
-      "Started port forwarding helper (%s)", filename);
-#else
+
     log_info(LD_GENERAL,
-      "Started port forwarding helper (%s) with pid %d", filename,
-      child_handle.pid);
-#endif
+             "Started port forwarding helper (%s) with pid '%d'",
+             filename, tor_process_get_pid(child_handle));
   }
 
   /* If child is running, read from its stdout and stderr) */
-  if (PROCESS_STATUS_RUNNING == child_handle.status) {
+  if (child_handle && PROCESS_STATUS_RUNNING == child_handle->status) {
     /* Read from stdout/stderr and log result */
     retval = 0;
 #ifdef MS_WINDOWS
-    stdout_status = log_from_handle(child_handle.stdout_pipe, LOG_INFO);
-    stderr_status = log_from_handle(child_handle.stderr_pipe, LOG_WARN);
+    stdout_status = log_from_handle(child_handle->stdout_pipe, LOG_INFO);
+    stderr_status = log_from_handle(child_handle->stderr_pipe, LOG_WARN);
     /* If we got this far (on Windows), the process started */
     retval = 0;
 #else
-    stdout_status = log_from_pipe(child_handle.stdout_handle,
+    stdout_status = log_from_pipe(child_handle->stdout_handle,
                     LOG_INFO, filename, &retval);
-    stderr_status = log_from_pipe(child_handle.stderr_handle,
+    stderr_status = log_from_pipe(child_handle->stderr_handle,
                     LOG_WARN, filename, &retval);
 #endif
     if (retval) {
@@ -4046,7 +4154,7 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
       /* There was a failure */
       retval = -1;
 #ifdef MS_WINDOWS
-    else if (tor_get_exit_code(child_handle, 0, NULL) !=
+    else if (!child_handle || tor_get_exit_code(child_handle, 0, NULL) !=
              PROCESS_EXIT_RUNNING) {
       /* process has exited or there was an error */
       /* TODO: Do something with the process return value */
@@ -4069,10 +4177,10 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
     if (0 != retval) {
       if (1 == retval) {
         log_info(LD_GENERAL, "Port forwarding helper terminated");
-        child_handle.status = PROCESS_STATUS_NOTRUNNING;
+        child_handle->status = PROCESS_STATUS_NOTRUNNING;
       } else {
         log_warn(LD_GENERAL, "Failed to read from port forwarding helper");
-        child_handle.status = PROCESS_STATUS_ERROR;
+        child_handle->status = PROCESS_STATUS_ERROR;
       }
 
       /* TODO: The child might not actually be finished (maybe it failed or