All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brandon Williams <bmwill@google.com>
To: git@vger.kernel.org
Cc: Brandon Williams <bmwill@google.com>, jrnieder@gmail.com, e@80x24.org
Subject: [PATCH v2 2/6] run-command: prepare command before forking
Date: Thu, 13 Apr 2017 11:32:48 -0700	[thread overview]
Message-ID: <20170413183252.4713-3-bmwill@google.com> (raw)
In-Reply-To: <20170413183252.4713-1-bmwill@google.com>

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


  parent reply	other threads:[~2017-04-13 18:35 UTC|newest]

Thread overview: 140+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-10 23:49 [PATCH 0/5] forking and threading Brandon Williams
2017-04-10 23:49 ` [PATCH 1/5] run-command: convert sane_execvp to sane_execvpe Brandon Williams
2017-04-12 19:22   ` Brandon Williams
2017-04-10 23:49 ` [PATCH 2/5] run-command: prepare argv before forking Brandon Williams
2017-04-10 23:49 ` [PATCH 3/5] run-command: allocate child_err " Brandon Williams
2017-04-10 23:49 ` [PATCH 4/5] run-command: prepare child environment " Brandon Williams
2017-04-11  0:58   ` Jonathan Nieder
2017-04-11 17:27     ` Brandon Williams
2017-04-11 17:30       ` Jonathan Nieder
2017-04-10 23:49 ` [PATCH 5/5] run-command: add note about forking and threading Brandon Williams
2017-04-11  0:26   ` Jonathan Nieder
2017-04-11  0:53     ` Eric Wong
2017-04-11 17:33       ` Jonathan Nieder
2017-04-11 17:34       ` Brandon Williams
2017-04-11 17:40         ` Eric Wong
2017-04-11  7:05 ` [PATCH 6/5] run-command: avoid potential dangers in forked child Eric Wong
2017-04-11 16:29   ` Brandon Williams
2017-04-11 16:59     ` Eric Wong
2017-04-11 17:17       ` Brandon Williams
2017-04-11 17:37 ` [PATCH 0/5] forking and threading Jonathan Nieder
2017-04-11 17:54   ` Brandon Williams
2017-04-13 18:32 ` [PATCH v2 0/6] " Brandon Williams
2017-04-13 18:32   ` [PATCH v2 1/6] t5550: use write_script to generate post-update hook Brandon Williams
2017-04-13 20:43     ` Jonathan Nieder
2017-04-13 20:59       ` Eric Wong
2017-04-13 21:35         ` Brandon Williams
2017-04-13 21:39           ` Eric Wong
2017-04-13 18:32   ` Brandon Williams [this message]
2017-04-13 21:14     ` [PATCH v2 2/6] run-command: prepare command before forking Jonathan Nieder
2017-04-13 22:41       ` Brandon Williams
2017-04-13 18:32   ` [PATCH v2 3/6] run-command: prepare child environment " Brandon Williams
2017-04-13 18:32   ` [PATCH v2 4/6] run-command: don't die in child when duping /dev/null Brandon Williams
2017-04-13 19:29     ` Eric Wong
2017-04-13 19:43       ` Brandon Williams
2017-04-13 18:32   ` [PATCH v2 5/6] run-command: eliminate calls to error handling functions in child Brandon Williams
2017-04-13 18:32   ` [PATCH v2 6/6] run-command: add note about forking and threading Brandon Williams
2017-04-13 20:50   ` [PATCH v2 0/6] " Jonathan Nieder
2017-04-13 23:44     ` Brandon Williams
2017-04-13 21:14   ` [PATCH 7/6] run-command: block signals between fork and execve Eric Wong
2017-04-13 23:37     ` Brandon Williams
2017-04-14  2:42     ` Brandon Williams
2017-04-14  5:26       ` Eric Wong
2017-04-14  5:35         ` Eric Wong
2017-04-14 16:58   ` [PATCH v3 00/10] forking and threading Brandon Williams
2017-04-14 16:58     ` [PATCH v3 01/10] t5550: use write_script to generate post-update hook Brandon Williams
2017-04-14 16:58     ` [PATCH v3 02/10] t0061: run_command executes scripts without a #! line Brandon Williams
2017-04-14 16:58     ` [PATCH v3 03/10] run-command: prepare command before forking Brandon Williams
2017-04-14 16:58     ` [PATCH v3 04/10] run-command: use the async-signal-safe execv instead of execvp Brandon Williams
2017-04-14 16:58     ` [PATCH v3 05/10] run-command: prepare child environment before forking Brandon Williams
2017-04-14 16:58     ` [PATCH v3 06/10] run-command: don't die in child when duping /dev/null Brandon Williams
2017-04-14 19:38       ` Eric Wong
2017-04-14 20:19         ` Brandon Williams
2017-04-14 16:58     ` [PATCH v3 07/10] run-command: eliminate calls to error handling functions in child Brandon Williams
2017-04-14 18:50       ` Eric Wong
2017-04-14 20:22         ` Brandon Williams
2017-04-14 16:59     ` [PATCH v3 08/10] run-command: handle dup2 and close errors " Brandon Williams
2017-04-14 16:59     ` [PATCH v3 09/10] run-command: add note about forking and threading Brandon Williams
2017-04-14 16:59     ` [PATCH v3 10/10] run-command: block signals between fork and execve Brandon Williams
2017-04-14 20:24       ` Brandon Williams
2017-04-14 21:35         ` Eric Wong
2017-04-17 22:08     ` [PATCH v4 00/10] forking and threading Brandon Williams
2017-04-17 22:08       ` [PATCH v4 01/10] t5550: use write_script to generate post-update hook Brandon Williams
2017-04-17 22:08       ` [PATCH v4 02/10] t0061: run_command executes scripts without a #! line Brandon Williams
2017-04-17 22:08       ` [PATCH v4 03/10] run-command: prepare command before forking Brandon Williams
2017-04-17 22:08       ` [PATCH v4 04/10] run-command: use the async-signal-safe execv instead of execvp Brandon Williams
2017-04-17 22:08       ` [PATCH v4 05/10] run-command: prepare child environment before forking Brandon Williams
2017-04-18  0:26         ` Eric Wong
2017-04-18 21:02           ` Brandon Williams
2017-04-17 22:08       ` [PATCH v4 06/10] run-command: don't die in child when duping /dev/null Brandon Williams
2017-04-17 22:08       ` [PATCH v4 07/10] run-command: eliminate calls to error handling functions in child Brandon Williams
2017-04-17 22:08       ` [PATCH v4 08/10] run-command: handle dup2 and close errors " Brandon Williams
2017-04-17 22:08       ` [PATCH v4 09/10] run-command: add note about forking and threading Brandon Williams
2017-04-17 22:08       ` [PATCH v4 10/10] run-command: block signals between fork and execve Brandon Williams
2017-04-18 23:17       ` [PATCH v5 00/11] forking and threading Brandon Williams
2017-04-18 23:17         ` [PATCH v5 01/11] t5550: use write_script to generate post-update hook Brandon Williams
2017-04-18 23:17         ` [PATCH v5 02/11] t0061: run_command executes scripts without a #! line Brandon Williams
2017-04-19  5:43           ` Johannes Sixt
2017-04-19  6:21             ` Johannes Sixt
2017-04-19 15:56               ` Brandon Williams
2017-04-19 18:18                 ` Johannes Sixt
2017-04-20 10:47                 ` Johannes Schindelin
2017-04-20 17:02                   ` Brandon Williams
2017-04-20 20:24                     ` Johannes Schindelin
2017-04-20 20:49                       ` Brandon Williams
2017-04-18 23:17         ` [PATCH v5 03/11] run-command: prepare command before forking Brandon Williams
2017-04-18 23:17         ` [PATCH v5 04/11] run-command: use the async-signal-safe execv instead of execvp Brandon Williams
2017-04-18 23:17         ` [PATCH v5 05/11] string-list: add string_list_remove function Brandon Williams
2017-04-18 23:31           ` Stefan Beller
2017-04-18 23:36             ` Brandon Williams
2017-04-18 23:40               ` Stefan Beller
2017-04-18 23:18         ` [PATCH v5 06/11] run-command: prepare child environment before forking Brandon Williams
2017-04-18 23:18         ` [PATCH v5 07/11] run-command: don't die in child when duping /dev/null Brandon Williams
2017-04-18 23:18         ` [PATCH v5 08/11] run-command: eliminate calls to error handling functions in child Brandon Williams
2017-04-18 23:18         ` [PATCH v5 09/11] run-command: handle dup2 and close errors " Brandon Williams
2017-04-18 23:18         ` [PATCH v5 10/11] run-command: add note about forking and threading Brandon Williams
2017-04-18 23:18         ` [PATCH v5 11/11] run-command: block signals between fork and execve Brandon Williams
2017-04-19  6:00           ` Johannes Sixt
2017-04-19  7:48             ` Eric Wong
2017-04-19 16:10               ` Brandon Williams
2017-04-19 23:13         ` [PATCH v6 00/11] forking and threading Brandon Williams
2017-04-19 23:13           ` [PATCH v6 01/11] t5550: use write_script to generate post-update hook Brandon Williams
2017-04-19 23:13           ` [PATCH v6 02/11] t0061: run_command executes scripts without a #! line Brandon Williams
2017-04-20 10:49             ` Johannes Schindelin
2017-04-20 16:58               ` Brandon Williams
2017-04-19 23:13           ` [PATCH v6 03/11] run-command: prepare command before forking Brandon Williams
2017-04-19 23:13           ` [PATCH v6 04/11] run-command: use the async-signal-safe execv instead of execvp Brandon Williams
2017-05-17  2:15             ` Junio C Hamano
2017-05-17  2:26               ` Jeff King
2017-05-17  2:28                 ` Jeff King
2017-05-17  3:41                 ` Junio C Hamano
2017-05-17 14:52                 ` Brandon Williams
2017-04-19 23:13           ` [PATCH v6 05/11] string-list: add string_list_remove function Brandon Williams
2017-04-19 23:13           ` [PATCH v6 06/11] run-command: prepare child environment before forking Brandon Williams
2017-04-19 23:13           ` [PATCH v6 07/11] run-command: don't die in child when duping /dev/null Brandon Williams
2017-04-19 23:13           ` [PATCH v6 08/11] run-command: eliminate calls to error handling functions in child Brandon Williams
2017-04-19 23:13           ` [PATCH v6 09/11] run-command: handle dup2 and close errors " Brandon Williams
2017-04-19 23:13           ` [PATCH v6 10/11] run-command: add note about forking and threading Brandon Williams
2017-04-19 23:13           ` [PATCH v6 11/11] run-command: block signals between fork and execve Brandon Williams
2017-04-24 22:37           ` [PATCH v6 00/11] forking and threading Brandon Williams
2017-04-24 23:50             ` [PATCH v6 12/11] run-command: don't try to execute directories Brandon Williams
2017-04-25  0:17               ` Jonathan Nieder
2017-04-25  1:58                 ` Junio C Hamano
2017-04-25  2:51                   ` Jonathan Nieder
2017-04-25  2:56                 ` Jeff King
2017-04-25  1:47               ` Junio C Hamano
2017-04-25  2:57               ` Jonathan Nieder
2017-04-25 17:54               ` [PATCH v7 1/2] exec_cmd: expose is_executable function Brandon Williams
2017-04-25 17:54                 ` [PATCH v7 2/2] run-command: don't try to execute directories Brandon Williams
2017-04-25 18:51                   ` Jonathan Nieder
2017-04-25 19:32                     ` Brandon Williams
2017-04-25 18:04                 ` [PATCH v7 1/2] exec_cmd: expose is_executable function Jonathan Nieder
2017-04-25 18:18                 ` Johannes Sixt
2017-04-25 18:38                   ` Brandon Williams
2017-04-25 23:46                 ` [PATCH v8 1/2] run-command: " Brandon Williams
2017-04-25 23:47                   ` [PATCH v8 2/2] run-command: restrict PATH search to executable files Brandon Williams
2017-04-25 23:50                     ` Jonathan Nieder
2017-04-26  1:44                     ` Junio C Hamano
2017-04-26 17:10                       ` [PATCH v9 " Brandon Williams
2017-04-27  0:33                         ` Junio C Hamano
2017-04-25 23:48                   ` [PATCH v8 1/2] run-command: expose is_executable function Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170413183252.4713-3-bmwill@google.com \
    --to=bmwill@google.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.