From d14acc97be453b6d5dd08e3cb6b5a438b048f4a5 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Fri, 14 Oct 2022 11:17:45 +0100
Subject: [PATCH] lib: Avoid calling setenv between fork and exec

setenv can call malloc and is not safe to call here.  Glibc is usually
tolerant of this and we haven't had problems before, but if you use
GLIBC_TUNABLES glibc.malloc.check=1 (or any alternate malloc / libc
which serializes) then you would see hangs if starting multiple
libguestfs handles from different threads at the same time.

This commit also updates the common submodule to pick up:

  commit 3c64bcdeaf684f05f46f3928b55aadafdfe72720
  Author: Richard W.M. Jones <rjones@redhat.com>
  Date:   Fri Oct 14 11:07:21 2022 +0100

    utils: Add function for copying the environment and adding new entries

    libguestfs is currently calling setenv at an unsafe location between
    fork and exec.  To fix this we need a way to copy and modify the
    environment before fork and then we can pass the modified environ to
    execve-like functions.  nbdkit already does the same so use that code.

    This function is copied and adapted from here under a compatible license:
    https://gitlab.com/nbdkit/nbdkit/-/blob/master/common/utils/environ.c

Thanks: Siddhesh Poyarekar
(cherry picked from commit e1c9bbb3d1d5ef81490977060120dda0963eb567)
---
 common              |  2 +-
 lib/command.c       | 28 +++++++++++++++++++++-------
 lib/launch-direct.c | 15 +++++++++++----
 3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/common b/common
index 4b4a5b846..3c64bcdea 160000
--- a/common
+++ b/common
@@ -1 +1 @@
-Subproject commit 4b4a5b84647b1496d034bcdff910930ca5f5c486
+Subproject commit 3c64bcdeaf684f05f46f3928b55aadafdfe72720
diff --git a/lib/command.c b/lib/command.c
index f2161de9a..abd79df23 100644
--- a/lib/command.c
+++ b/lib/command.c
@@ -452,13 +452,15 @@ debug_command (struct command *cmd)
   }
 }
 
-static void run_child (struct command *cmd) __attribute__((noreturn));
+static void run_child (struct command *cmd,
+                       char **env) __attribute__((noreturn));
 
 static int
 run_command (struct command *cmd)
 {
   int errorfd[2] = { -1, -1 };
   int outfd[2] = { -1, -1 };
+  CLEANUP_FREE_STRING_LIST char **env = NULL;
 
   /* Set up a pipe to capture command output and send it to the error log. */
   if (cmd->capture_errors) {
@@ -476,6 +478,10 @@ run_command (struct command *cmd)
     }
   }
 
+  env = guestfs_int_copy_environ (environ, "LC_ALL", "C", NULL);
+  if (env == NULL)
+    goto error;
+
   cmd->pid = fork ();
   if (cmd->pid == -1) {
     perrorf (cmd->g, "fork");
@@ -519,7 +525,7 @@ run_command (struct command *cmd)
   if (cmd->stderr_to_stdout)
     dup2 (1, 2);
 
-  run_child (cmd);
+  run_child (cmd, env);
   /*NOTREACHED*/
 
  error:
@@ -536,7 +542,7 @@ run_command (struct command *cmd)
 }
 
 static void
-run_child (struct command *cmd)
+run_child (struct command *cmd, char **env)
 {
   struct sigaction sa;
   int i, err, fd, max_fd, r;
@@ -571,9 +577,6 @@ run_child (struct command *cmd)
       close (fd);
   }
 
-  /* Clean up the environment. */
-  setenv ("LC_ALL", "C", 1);
-
   /* Set the umask for all subcommands to something sensible (RHBZ#610880). */
   umask (022);
 
@@ -610,6 +613,12 @@ run_child (struct command *cmd)
    * tests/regressions/test-big-heap.c
    */
 
+  /* Note the assignment of environ avoids using execvpe which is a
+   * GNU extension.  See also:
+   * https://github.com/libguestfs/libnbd/commit/dc64ac5cdd0bc80ca4e18935ad0e8801d11a8644
+   */
+  environ = env;
+
   /* Run the command. */
   switch (cmd->style) {
   case COMMAND_STYLE_EXECV:
@@ -792,6 +801,7 @@ guestfs_int_cmd_pipe_run (struct command *cmd, const char *mode)
   int errfd = -1;
   int r_mode;
   int ret;
+  CLEANUP_FREE_STRING_LIST char **env = NULL;
 
   finish_command (cmd);
 
@@ -826,6 +836,10 @@ guestfs_int_cmd_pipe_run (struct command *cmd, const char *mode)
     goto error;
   }
 
+  env = guestfs_int_copy_environ (environ, "LC_ALL", "C", NULL);
+  if (env == NULL)
+    goto error;
+
   cmd->pid = fork ();
   if (cmd->pid == -1) {
     perrorf (cmd->g, "fork");
@@ -864,7 +878,7 @@ guestfs_int_cmd_pipe_run (struct command *cmd, const char *mode)
     close (fd[0]);
   }
 
-  run_child (cmd);
+  run_child (cmd, env);
   /*NOTREACHED*/
 
  error:
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index b292b9c26..acb86c6cc 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -390,6 +390,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   const char *cpu_model;
   CLEANUP_FREE char *append = NULL;
   CLEANUP_FREE_STRING_LIST char **argv = NULL;
+  CLEANUP_FREE_STRING_LIST char **env = NULL;
 
   if (!g->nr_drives) {
     error (g, _("you must call guestfs_add_drive before guestfs_launch"));
@@ -726,6 +727,15 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   /* Get the argv list from the command line. */
   argv = qemuopts_to_argv (qopts);
 
+  /* Create the environ for the child process. */
+  env = guestfs_int_copy_environ (environ,
+                                  "LC_ALL", "C",
+                                  /* Prevents qemu opening /dev/dsp */
+                                  "QEMU_AUDIO_DRV", "none",
+                                  NULL);
+  if (env == NULL)
+    goto cleanup0;
+
   r = fork ();
   if (r == -1) {
     perrorf (g, "fork");
@@ -790,12 +800,9 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
     if (g->pgroup)
       setpgid (0, 0);
 
-    setenv ("LC_ALL", "C", 1);
-    setenv ("QEMU_AUDIO_DRV", "none", 1); /* Prevents qemu opening /dev/dsp */
-
     TRACE0 (launch_run_qemu);
 
-    execv (g->hv, argv);        /* Run qemu. */
+    execve (g->hv, argv, env);        /* Run qemu. */
     perror (g->hv);
     _exit (EXIT_FAILURE);
   }