[PATCH v2 2/6] run-command: prepare command before forking

[Brandon Williams]

In order to avoid allocation between 'fork()' and 'exec()' the argv
array used in the exec call is prepared prior to forking the process.

In addition to this, the function used to exec is changed from
'execvp()' to 'execv()' as the (p) variant of exec has the potential to
call malloc during the path resolution it performs.  Instead we simply
do the path resolution ourselves during the preparation stage prior to
forking.

Signed-off-by: Brandon Williams <bmw...@google.com>
---
 run-command.c | 60 +++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/run-command.c b/run-command.c
index 574b81d3e..9ee9fde97 100644
--- a/run-command.c
+++ b/run-command.c
@@ -221,18 +221,6 @@ static const char **prepare_shell_cmd(struct argv_array 
*out, const char **argv)
 }
 
 #ifndef GIT_WINDOWS_NATIVE
-static int execv_shell_cmd(const char **argv)
-{
-       struct argv_array nargv = ARGV_ARRAY_INIT;
-       prepare_shell_cmd(&nargv, argv);
-       trace_argv_printf(nargv.argv, "trace: exec:");
-       sane_execvp(nargv.argv[0], (char **)nargv.argv);
-       argv_array_clear(&nargv);
-       return -1;
-}
-#endif
-
-#ifndef GIT_WINDOWS_NATIVE
 static int child_notifier = -1;
 
 static void notify_parent(void)
@@ -244,6 +232,35 @@ static void notify_parent(void)
         */
        xwrite(child_notifier, "", 1);
 }
+
+static void prepare_cmd(struct argv_array *out, const struct child_process 
*cmd)
+{
+       if (!cmd->argv[0])
+               die("BUG: command is empty");
+
+       if (cmd->git_cmd) {
+               argv_array_push(out, "git");
+               argv_array_pushv(out, cmd->argv);
+       } else if (cmd->use_shell) {
+               prepare_shell_cmd(out, cmd->argv);
+       } else {
+               argv_array_pushv(out, cmd->argv);
+       }
+
+       /*
+        * If there are no '/' characters in the command then perform a path
+        * lookup and use the resolved path as the command to exec.  If there
+        * are no '/' characters or if the command wasn't found in the path,
+        * have exec attempt to invoke the command directly.
+        */
+       if (!strchr(out->argv[0], '/')) {
+               char *program = locate_in_PATH(out->argv[0]);
+               if (program) {
+                       free((char *)out->argv[0]);
+                       out->argv[0] = program;
+               }
+       }
+}
 #endif
 
 static inline void set_cloexec(int fd)
@@ -372,9 +389,13 @@ int start_command(struct child_process *cmd)
 #ifndef GIT_WINDOWS_NATIVE
 {
        int notify_pipe[2];
+       struct argv_array argv = ARGV_ARRAY_INIT;
+
        if (pipe(notify_pipe))
                notify_pipe[0] = notify_pipe[1] = -1;
 
+       prepare_cmd(&argv, cmd);
+
        cmd->pid = fork();
        failed_errno = errno;
        if (!cmd->pid) {
@@ -437,12 +458,9 @@ int start_command(struct child_process *cmd)
                                        unsetenv(*cmd->env);
                        }
                }
-               if (cmd->git_cmd)
-                       execv_git_cmd(cmd->argv);
-               else if (cmd->use_shell)
-                       execv_shell_cmd(cmd->argv);
-               else
-                       sane_execvp(cmd->argv[0], (char *const*) cmd->argv);
+
+               execv(argv.argv[0], (char *const *) argv.argv);
+
                if (errno == ENOENT) {
                        if (!cmd->silent_exec_failure)
                                error("cannot run %s: %s", cmd->argv[0],
@@ -458,7 +476,7 @@ int start_command(struct child_process *cmd)
                mark_child_for_cleanup(cmd->pid, cmd);
 
        /*
-        * Wait for child's execvp. If the execvp succeeds (or if fork()
+        * Wait for child's exec. If the exec succeeds (or if fork()
         * failed), EOF is seen immediately by the parent. Otherwise, the
         * child process sends a single byte.
         * Note that use of this infrastructure is completely advisory,
@@ -467,7 +485,7 @@ int start_command(struct child_process *cmd)
        close(notify_pipe[1]);
        if (read(notify_pipe[0], &notify_pipe[1], 1) == 1) {
                /*
-                * At this point we know that fork() succeeded, but execvp()
+                * At this point we know that fork() succeeded, but exec()
                 * failed. Errors have been reported to our stderr.
                 */
                wait_or_whine(cmd->pid, cmd->argv[0], 0);
@@ -475,6 +493,8 @@ int start_command(struct child_process *cmd)
                cmd->pid = -1;
        }
        close(notify_pipe[0]);
+
+       argv_array_clear(&argv);
 }
 #else
 {
-- 
2.12.2.762.g0e3151a226-goog