run-command: prepare command before forking

According to [1] we need to only call async-signal-safe operations between fork and exec. Using malloc to build the argv array isn't async-signal-safe. In order to avoid allocation between 'fork()' and 'exec()' prepare the argv array used in the exec call prior to forking the process. [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/fork.html Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Brandon Williams committed Apr 19, 2017 at 16:13 UTC 3967e25be11ab96ced71f16b9f082de270a518db
1 file changed +26 -20
run-command.c
+26 -20
@@ -220,18 +220,6 @@ static const char **prepare_shell_cmd(struct argv_array *out, const char **argv)
220 return out->argv;
221 }
222
223 -#ifndef GIT_WINDOWS_NATIVE
224 -static int execv_shell_cmd(const char **argv)
225 -{
226 - struct argv_array nargv = ARGV_ARRAY_INIT;
227 - prepare_shell_cmd(&nargv, argv);
228 - trace_argv_printf(nargv.argv, "trace: exec:");
229 - sane_execvp(nargv.argv[0], (char **)nargv.argv);
230 - argv_array_clear(&nargv);
231 - return -1;
232 -}
233 -#endif
234 -
223 #ifndef GIT_WINDOWS_NATIVE
224 static int child_notifier = -1;
225
@@ -244,6 +232,21 @@ static void notify_parent(void)
232 */
233 xwrite(child_notifier, "", 1);
234 }
235 +
236 +static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)
237 +{
238 + if (!cmd->argv[0])
239 + die("BUG: command is empty");
240 +
241 + if (cmd->git_cmd) {
242 + argv_array_push(out, "git");
243 + argv_array_pushv(out, cmd->argv);
244 + } else if (cmd->use_shell) {
245 + prepare_shell_cmd(out, cmd->argv);
246 + } else {
247 + argv_array_pushv(out, cmd->argv);
248 + }
249 +}
250 #endif
251
252 static inline void set_cloexec(int fd)
@@ -372,9 +375,13 @@ fail_pipe:
375 #ifndef GIT_WINDOWS_NATIVE
376 {
377 int notify_pipe[2];
378 + struct argv_array argv = ARGV_ARRAY_INIT;
379 +
380 if (pipe(notify_pipe))
381 notify_pipe[0] = notify_pipe[1] = -1;
382
383 + prepare_cmd(&argv, cmd);
384 +
385 cmd->pid = fork();
386 failed_errno = errno;
387 if (!cmd->pid) {
@@ -437,12 +444,9 @@ fail_pipe:
444 unsetenv(*cmd->env);
445 }
446 }
440 - if (cmd->git_cmd)
441 - execv_git_cmd(cmd->argv);
442 - else if (cmd->use_shell)
443 - execv_shell_cmd(cmd->argv);
444 - else
445 - sane_execvp(cmd->argv[0], (char *const*) cmd->argv);
447 +
448 + sane_execvp(argv.argv[0], (char *const *) argv.argv);
449 +
450 if (errno == ENOENT) {
451 if (!cmd->silent_exec_failure)
452 error("cannot run %s: %s", cmd->argv[0],
@@ -458,7 +462,7 @@ fail_pipe:
462 mark_child_for_cleanup(cmd->pid, cmd);
463
464 /*
461 - * Wait for child's execvp. If the execvp succeeds (or if fork()
465 + * Wait for child's exec. If the exec succeeds (or if fork()
466 * failed), EOF is seen immediately by the parent. Otherwise, the
467 * child process sends a single byte.
468 * Note that use of this infrastructure is completely advisory,
@@ -467,7 +471,7 @@ fail_pipe:
471 close(notify_pipe[1]);
472 if (read(notify_pipe[0], &notify_pipe[1], 1) == 1) {
473 /*
470 - * At this point we know that fork() succeeded, but execvp()
474 + * At this point we know that fork() succeeded, but exec()
475 * failed. Errors have been reported to our stderr.
476 */
477 wait_or_whine(cmd->pid, cmd->argv[0], 0);
@@ -475,6 +479,8 @@ fail_pipe:
479 cmd->pid = -1;
480 }
481 close(notify_pipe[0]);
482 +
483 + argv_array_clear(&argv);
484 }
485 #else
486 {