-LRN: Another take on std descriptor inheritance

Now descriptors are not inherited by default, you have to pass
a set of flags to make it so. When pipes are given, flags have
no effect.
gnunet-arm now has two options to block stdout and stderr from being
passed to gnunet-service-arm

7 files changed, 105 insertions, 23 deletions

diff --git a/src/util/crypto_random.c b/src/util/crypto_random.c
index e8522d5e7..dbf71d78a 100644
--- a/src/util/crypto_random.c
+++ b/src/util/crypto_random.c
@@ -284,7 +284,7 @@ entropy_generator (void *cls, const char *what, int printchar, int current,
   LOG (GNUNET_ERROR_TYPE_INFO, _("Starting `%s' process to generate entropy\n"),
        "find");
   genproc =
-     GNUNET_OS_start_process (GNUNET_NO,
+     GNUNET_OS_start_process (GNUNET_NO, 0, 
                               NULL, NULL, "sh", "sh", "-c",
                               "exec find / -mount -type f -exec cp {} /dev/null \\; 2>/dev/null",
                               NULL);
diff --git a/src/util/gnunet-uri.c b/src/util/gnunet-uri.c
index 6268360f2..8cc813be5 100644
--- a/src/util/gnunet-uri.c
+++ b/src/util/gnunet-uri.c
@@ -118,7 +118,7 @@ run (void *cls, char *const *args, const char *cfgfile,
                                        GNUNET_DISK_pipe_handle (sigpipe,
                                                                 GNUNET_DISK_PIPE_END_READ),
                                        &maint_child_death, NULL);   
-  p = GNUNET_OS_start_process (GNUNET_NO,
+  p = GNUNET_OS_start_process (GNUNET_NO, 0,
                                NULL, NULL,
                                program,
                                program,
diff --git a/src/util/helper.c b/src/util/helper.c
index 6a84ff3ef..64f082bee 100644
--- a/src/util/helper.c
+++ b/src/util/helper.c
@@ -314,7 +314,7 @@ start_helper (struct GNUNET_HELPER_Handle *h)
   h->fh_to_helper =
       GNUNET_DISK_pipe_handle (h->helper_in, GNUNET_DISK_PIPE_END_WRITE);
   h->helper_proc =
-      GNUNET_OS_start_process_vap (GNUNET_NO,
+      GNUNET_OS_start_process_vap (GNUNET_NO, 0, 
                                    h->helper_in, h->helper_out,
                                    h->binary_name,
                                    h->binary_argv);
diff --git a/src/util/os_priority.c b/src/util/os_priority.c
index c9174dee9..3a10599b5 100644
--- a/src/util/os_priority.c
+++ b/src/util/os_priority.c
@@ -768,6 +768,10 @@ CreateCustomEnvTable (char **vars)
  * Start a process.
  *
  * @param pipe_control should a pipe be used to send signals to the child?
+ * @param std_inheritance a set of GNUNET_OS_INHERIT_STD_* flags controlling which
+ *        std handles of the parent are inherited by the child.
+ *        pipe_stdin and pipe_stdout take priority over std_inheritance
+ *        (when they are non-NULL).
  * @param pipe_stdin pipe to use to send input to child process (or NULL)
  * @param pipe_stdout pipe to use to get output from child process (or NULL)
  * @param lsocks array of listen sockets to dup systemd-style (or NULL);
@@ -778,6 +782,7 @@ CreateCustomEnvTable (char **vars)
  */
 static struct GNUNET_OS_Process *
 start_process (int pipe_control,
+               enum GNUNET_OS_InheritStdioFlags std_inheritance,
                struct GNUNET_DISK_PipeHandle *pipe_stdin,
                struct GNUNET_DISK_PipeHandle *pipe_stdout,
                const SOCKTYPE *lsocks,
@@ -870,6 +875,10 @@ start_process (int pipe_control,
       LOG_STRERROR (GNUNET_ERROR_TYPE_ERROR, "dup2");
     GNUNET_break (0 == close (fd_stdout_write));
   }
+  else if (!(std_inheritance & GNUNET_OS_INHERIT_STD_OUT))
+  {
+    close (1);
+  }
   if (pipe_stdin != NULL)
   {
 
@@ -878,6 +887,14 @@ start_process (int pipe_control,
       LOG_STRERROR (GNUNET_ERROR_TYPE_ERROR, "dup2");
     GNUNET_break (0 == close (fd_stdin_read));
   }
+  else if (!(std_inheritance & GNUNET_OS_INHERIT_STD_IN))
+  {
+    close (0);
+  }
+  if (!(std_inheritance & GNUNET_OS_INHERIT_STD_ERR))
+  {
+    close (2);
+  }
   if (lscp != NULL)
   {
     /* read systemd documentation... */
@@ -961,6 +978,10 @@ start_process (int pipe_control,
   long lRet;
   HANDLE stdin_handle;
   HANDLE stdout_handle;
+  HANDLE stdih, stdoh, stdeh;
+  DWORD stdif, stdof, stdef;
+  BOOL bresult;
+  DWORD error_code;
 
   if (GNUNET_SYSERR == GNUNET_OS_check_helper_binary (filename))
     return NULL; /* not executable */
@@ -1083,9 +1104,11 @@ start_process (int pipe_control,
 
   memset (&start, 0, sizeof (start));
   start.cb = sizeof (start);
-  if ((pipe_stdin != NULL) || (pipe_stdout != NULL))
+  if ((pipe_stdin != NULL) || (pipe_stdout != NULL) || (std_inheritance != 0))
     start.dwFlags |= STARTF_USESTDHANDLES;
 
+  stdih = GetStdHandle (STD_INPUT_HANDLE);
+  GetHandleInformation (stdih, &stdif);
   if (pipe_stdin != NULL)
   {
     GNUNET_DISK_internal_file_handle_ (GNUNET_DISK_pipe_handle
@@ -1093,7 +1116,21 @@ start_process (int pipe_control,
                                        &stdin_handle, sizeof (HANDLE));
     start.hStdInput = stdin_handle;
   }
+  if (stdih)
+  {
+    if (std_inheritance & GNUNET_OS_INHERIT_STD_IN)
+    {
+      SetHandleInformation (stdih, HANDLE_FLAG_INHERIT, 1);
+      if (pipe_stdin == NULL)
+        start.hStdInput = stdih;
+    }
+    else
+      SetHandleInformation (stdih, HANDLE_FLAG_INHERIT, 0);
+  }
+    
 
+  stdoh = GetStdHandle (STD_OUTPUT_HANDLE);
+  GetHandleInformation (stdoh, &stdof);
   if (pipe_stdout != NULL)
   {
     GNUNET_DISK_internal_file_handle_ (GNUNET_DISK_pipe_handle
@@ -1102,6 +1139,30 @@ start_process (int pipe_control,
                                        &stdout_handle, sizeof (HANDLE));
     start.hStdOutput = stdout_handle;
   }
+  if (stdoh)
+  {
+    if (std_inheritance & GNUNET_OS_INHERIT_STD_OUT)
+    {
+      SetHandleInformation (stdoh, HANDLE_FLAG_INHERIT, 1);
+      if (pipe_stdout == NULL)
+        start.hStdOutput = stdoh;
+    }
+    else
+      SetHandleInformation (stdoh, HANDLE_FLAG_INHERIT, 0);
+  }
+
+  stdeh = GetStdHandle (STD_ERROR_HANDLE);
+  GetHandleInformation (stdeh, &stdef);
+  if (stdeh)
+  {
+    if (std_inheritance & GNUNET_OS_INHERIT_STD_ERR)
+    {
+      SetHandleInformation (stdeh, HANDLE_FLAG_INHERIT, 1);
+      start.hStdError = stdeh;
+    }
+    else
+      SetHandleInformation (stdeh, HANDLE_FLAG_INHERIT, 0);
+  }
 
   if (GNUNET_YES == pipe_control)
   {
@@ -1179,24 +1240,36 @@ start_process (int pipe_control,
     return NULL;
   }
 
-  if (!CreateProcessW (wpath, wcmd, NULL, NULL, TRUE,
-       DETACHED_PROCESS | CREATE_SUSPENDED, env_block, NULL, &start, &proc))
+  bresult = CreateProcessW (wpath, wcmd, NULL, NULL, TRUE,
+       DETACHED_PROCESS | CREATE_SUSPENDED, env_block, NULL, &start, &proc);
+  error_code = GetLastError ();
+
+  if ((NULL == pipe_stdin) && (stdih))
+    SetHandleInformation (stdih, HANDLE_FLAG_INHERIT, stdif);
+    
+
+  if ((NULL == pipe_stdout) && (stdoh))
+    SetHandleInformation (stdoh, HANDLE_FLAG_INHERIT, stdof);
+
+  if (stdeh)
+    SetHandleInformation (stdeh, HANDLE_FLAG_INHERIT, stdef);
+
+  GNUNET_free (env_block);
+  GNUNET_free (cmd);
+  free (wpath);
+  free (wcmd);
+
+  if (!bresult)
   {
-    SetErrnoFromWinError (GetLastError ());
+    SetErrnoFromWinError (error_code);
     LOG_STRERROR (GNUNET_ERROR_TYPE_ERROR, "CreateProcess");
     if (NULL != control_pipe)
       GNUNET_DISK_file_close (control_pipe);
     if (NULL != lsocks)
       GNUNET_DISK_pipe_close (lsocks_pipe);
-    GNUNET_free (env_block);
-    GNUNET_free (cmd);
-    free (wpath);
-    free (wcmd);
     return NULL;
   }
 
-  GNUNET_free (env_block);
-
   gnunet_proc = GNUNET_malloc (sizeof (struct GNUNET_OS_Process));
   gnunet_proc->pid = proc.dwProcessId;
   gnunet_proc->handle = proc.hProcess;
@@ -1206,9 +1279,6 @@ start_process (int pipe_control,
 
   ResumeThread (proc.hThread);
   CloseHandle (proc.hThread);
-  GNUNET_free (cmd);
-  free (wpath);
-  free (wcmd);
 
   if (lsocks == NULL || lsocks[0] == INVALID_SOCKET)
     return gnunet_proc;
@@ -1300,6 +1370,7 @@ start_process (int pipe_control,
  * Start a process.
  *
  * @param pipe_control should a pipe be used to send signals to the child?
+ * @param std_inheritance a set of GNUNET_OS_INHERIT_STD_* flags
  * @param pipe_stdin pipe to use to send input to child process (or NULL)
  * @param pipe_stdout pipe to use to get output from child process (or NULL)
  * @param filename name of the binary
@@ -1308,12 +1379,14 @@ start_process (int pipe_control,
  */
 struct GNUNET_OS_Process *
 GNUNET_OS_start_process_vap (int pipe_control,
+                             enum GNUNET_OS_InheritStdioFlags std_inheritance,
                              struct GNUNET_DISK_PipeHandle *pipe_stdin,
                              struct GNUNET_DISK_PipeHandle *pipe_stdout,
                              const char *filename, 
                              char *const argv[])
 {
   return start_process (pipe_control,
+                        std_inheritance,
                         pipe_stdin,
                         pipe_stdout,
                         NULL,
@@ -1326,6 +1399,7 @@ GNUNET_OS_start_process_vap (int pipe_control,
  * Start a process.
  *
  * @param pipe_control should a pipe be used to send signals to the child?
+ * @param std_inheritance a set of GNUNET_OS_INHERIT_STD_* flags
  * @param pipe_stdin pipe to use to send input to child process (or NULL)
  * @param pipe_stdout pipe to use to get output from child process (or NULL)
  * @param filename name of the binary
@@ -1334,6 +1408,7 @@ GNUNET_OS_start_process_vap (int pipe_control,
  */
 struct GNUNET_OS_Process *
 GNUNET_OS_start_process_va (int pipe_control,
+                            enum GNUNET_OS_InheritStdioFlags std_inheritance,
                             struct GNUNET_DISK_PipeHandle *pipe_stdin,
                             struct GNUNET_DISK_PipeHandle *pipe_stdout,
                             const char *filename, va_list va)
@@ -1355,6 +1430,7 @@ GNUNET_OS_start_process_va (int pipe_control,
     argc++;
   va_end (ap);
   ret = GNUNET_OS_start_process_vap (pipe_control,
+                                     std_inheritance,
                                      pipe_stdin,
                                      pipe_stdout,
                                      filename,
@@ -1369,6 +1445,7 @@ GNUNET_OS_start_process_va (int pipe_control,
  * Start a process.
  *
  * @param pipe_control should a pipe be used to send signals to the child?
+ * @param std_inheritance a set of GNUNET_OS_INHERIT_STD_* flags
  * @param pipe_stdin pipe to use to send input to child process (or NULL)
  * @param pipe_stdout pipe to use to get output from child process (or NULL)
  * @param filename name of the binary
@@ -1379,6 +1456,7 @@ GNUNET_OS_start_process_va (int pipe_control,
  */
 struct GNUNET_OS_Process *
 GNUNET_OS_start_process (int pipe_control,
+                         enum GNUNET_OS_InheritStdioFlags std_inheritance,
                          struct GNUNET_DISK_PipeHandle *pipe_stdin,
                          struct GNUNET_DISK_PipeHandle *pipe_stdout,
                          const char *filename, ...)
@@ -1387,7 +1465,8 @@ GNUNET_OS_start_process (int pipe_control,
   va_list ap;
 
   va_start (ap, filename);
-  ret = GNUNET_OS_start_process_va (pipe_control, pipe_stdin, pipe_stdout, filename, ap);
+  ret = GNUNET_OS_start_process_va (pipe_control, std_inheritance, pipe_stdin,
+      pipe_stdout, filename, ap);
   va_end (ap);
   return ret;
 }
@@ -1405,11 +1484,13 @@ GNUNET_OS_start_process (int pipe_control,
  */
 struct GNUNET_OS_Process *
 GNUNET_OS_start_process_v (int pipe_control,
+                           enum GNUNET_OS_InheritStdioFlags std_inheritance,
                            const SOCKTYPE *lsocks,
                            const char *filename,
                            char *const argv[])
 {
   return start_process (pipe_control,
+                        std_inheritance,
                         NULL,
                         NULL,
                         lsocks,
@@ -1723,7 +1804,8 @@ GNUNET_OS_command_run (GNUNET_OS_LineProcessor proc, void *proc_cls,
   if (NULL == opipe)
     return NULL;
   va_start (ap, binary);
-  eip = GNUNET_OS_start_process_va (GNUNET_NO, NULL, opipe, binary, ap);
+  /* redirect stdout, don't inherit stderr/stdin */
+  eip = GNUNET_OS_start_process_va (GNUNET_NO, 0, NULL, opipe, binary, ap);
   va_end (ap);
   if (NULL == eip)
   {
diff --git a/src/util/test_common_logging_runtime_loglevels.c b/src/util/test_common_logging_runtime_loglevels.c
index d01cd3bce..4c6ae0039 100644
--- a/src/util/test_common_logging_runtime_loglevels.c
+++ b/src/util/test_common_logging_runtime_loglevels.c
@@ -317,7 +317,7 @@ runone ()
     break;
   }
 
-  proc = GNUNET_OS_start_process (GNUNET_NO, NULL, pipe_stdout,
+  proc = GNUNET_OS_start_process (GNUNET_NO, GNUNET_OS_INHERIT_STD_OUT_AND_ERR, NULL, pipe_stdout,
 #if MINGW
                                   "test_common_logging_dummy",
 #else
diff --git a/src/util/test_os_start_process.c b/src/util/test_os_start_process.c
index 3c30c75f3..e24e09500 100644
--- a/src/util/test_os_start_process.c
+++ b/src/util/test_os_start_process.c
@@ -131,7 +131,7 @@ run_task (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc)
   }
 
   proc =
-      GNUNET_OS_start_process (GNUNET_NO, hello_pipe_stdin, hello_pipe_stdout, fn,
+      GNUNET_OS_start_process (GNUNET_NO, GNUNET_OS_INHERIT_STD_ERR, hello_pipe_stdin, hello_pipe_stdout, fn,
                                "test_gnunet_echo_hello", "-", NULL);
   GNUNET_free (fn);
 
@@ -200,7 +200,7 @@ check_kill ()
     return 1;
   }
   proc =
-    GNUNET_OS_start_process (GNUNET_YES, hello_pipe_stdin, hello_pipe_stdout, fn,
+    GNUNET_OS_start_process (GNUNET_YES, GNUNET_OS_INHERIT_STD_ERR, hello_pipe_stdin, hello_pipe_stdout, fn,
                              "gnunet-service-resolver", "-", NULL); 
   sleep (1); /* give process time to start, so we actually use the pipe-kill mechanism! */
   if (0 != GNUNET_OS_process_kill (proc, SIGTERM))
@@ -233,7 +233,7 @@ check_instant_kill ()
     return 1;
   }
   proc =
-    GNUNET_OS_start_process (GNUNET_YES, hello_pipe_stdin, hello_pipe_stdout, fn,
+    GNUNET_OS_start_process (GNUNET_YES, GNUNET_OS_INHERIT_STD_ERR, hello_pipe_stdin, hello_pipe_stdout, fn,
                              "gnunet-service-resolver", "-", NULL); 
   if (0 != GNUNET_OS_process_kill (proc, SIGTERM))
   {
diff --git a/src/util/test_resolver_api.c b/src/util/test_resolver_api.c
index f924661dd..b9ca81b0f 100644
--- a/src/util/test_resolver_api.c
+++ b/src/util/test_resolver_api.c
@@ -370,7 +370,7 @@ main (int argc, char *argv[])
   pfx = GNUNET_OS_installation_get_path (GNUNET_OS_IPK_BINDIR);
   GNUNET_asprintf (&fn, "%s%cgnunet-service-resolver", pfx, DIR_SEPARATOR);
   GNUNET_free (pfx);
-  proc = GNUNET_OS_start_process (GNUNET_YES, NULL, NULL, fn, "gnunet-service-resolver",
+  proc = GNUNET_OS_start_process (GNUNET_YES, GNUNET_OS_INHERIT_STD_OUT_AND_ERR, NULL, NULL, fn, "gnunet-service-resolver",
                                   "-c", "test_resolver_api_data.conf", NULL);
   GNUNET_assert (NULL != proc);
   GNUNET_free (fn);