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 e1c9bbb3d1)
This commit is contained in:
Richard W.M. Jones
2022-10-14 11:17:45 +01:00
parent 0d47870a2b
commit d14acc97be
3 changed files with 33 additions and 12 deletions

2
common

Submodule common updated: 4b4a5b8464...3c64bcdeaf

View File

@@ -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:

View File

@@ -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);
}