Always restore global environ if we use vfork

We may change the global environ variable in the child; when using
vfork() this also sets it in the parent.  Preserve the parent's
environ in child_execute_job() so it takes effect for all callers.

Reported by Denis Excoffier <bug-tar@Denis-Excoffier.org>
Root cause found by Martin Dorey <Martin.Dorey@hitachivantara.com>

* src/job.c (start_job_command): Remove save/restore of the parent
environment.
(child_execute_job): Add save/restore of the parent environment,
if we use vfork().
* tests/scripts/functions/shell: Add a test the crashes if we don't
reset environ after we run $(shell ...).

diff --git a/src/job.c b/src/job.c
index d12a9138ccf9170d3567d7b1fab76f0a14f42f77..19f2797761c25b8f532c503acaa3103d7698fd42 100644 (file)
--- a/src/job.c
+++ b/src/job.c
@@ -1460,9 +1460,6 @@ start_job_command (struct child *child)
 #endif /* !VMS */
     {
       /* Fork the child process.  */
-
-      char **parent_environ;
-
     run_local:
       block_sigs ();
 
@@ -1473,14 +1470,11 @@ start_job_command (struct child *child)
 
 #else
 
-      parent_environ = environ;
-
       jobserver_pre_child (flags & COMMANDS_RECURSE);
 
       child->pid = child_execute_job ((struct childbase *)child,
                                       child->good_stdin, argv);
 
-      environ = parent_environ; /* Restore value child may have clobbered.  */
       jobserver_post_child (flags & COMMANDS_RECURSE);
 
 #endif /* !VMS */
@@ -1503,7 +1497,7 @@ start_job_command (struct child *child)
         char *cmdline = argv[0];
         /* We don't have a way to pass environment to 'system',
            so we need to save and restore ours, sigh...  */
-        char **parent_environ = environ;
+        char **parent_env = environ;
 
         environ = child->environment;
 
@@ -1518,7 +1512,7 @@ start_job_command (struct child *child)
 
         dos_command_running = 1;
         proc_return = system (cmdline);
-        environ = parent_environ;
+        environ = parent_env;
         execute_by_shell = 0;   /* for the next time */
       }
     else
@@ -2299,9 +2293,16 @@ child_execute_job (struct childbase *child, int good_stdin, char **argv)
 
 #if !defined(USE_POSIX_SPAWN)
 
-  pid = vfork();
-  if (pid != 0)
-    return pid;
+  {
+    /* The child may clobber environ so remember ours and restore it.  */
+    char **parent_env = environ;
+    pid = vfork ();
+    if (pid != 0)
+      {
+        environ = parent_env;
+        return pid;
+      }
+  }
 
   /* We are the child.  */
   unblock_all_sigs ();
@@ -2552,7 +2553,9 @@ exec_command (char **argv, char **envp)
     errno = ENOEXEC;
 
 # else
-  /* Run the program.  */
+
+  /* Run the program.  Don't use execvpe() as we want the search for argv[0]
+     to use the new PATH, but execvpe() searches before resetting PATH.  */
   environ = envp;
   execvp (argv[0], argv);
 

diff --git a/tests/scripts/functions/shell b/tests/scripts/functions/shell
index e889b5f74e5793413a9e7541795557e156742344..d89a0c831d70d16ceff35d8ddf333ebb5015af61 100644 (file)
--- a/tests/scripts/functions/shell
+++ b/tests/scripts/functions/shell
@@ -183,6 +183,15 @@ endif
 !,
                       '--no-print-directory -j2 --jobserver-style=pipe', "#MAKE#[2]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.\n: 2\n: 1");
     }
+
+    # This crashes if we use vfork and don't reset environ properly
+    run_make_test(q!
+export PATH = $(shell echo "tests:$$PATH")
+foo = $(shell echo yes)
+
+all:;echo $(foo)
+!,
+                  '', "echo yes\nyes\n");
 }
 
 # If we're not using pipes for jobserver, then they are available in sub-makes