git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Brandon Williams <bmwill@google.com>
To: git@vger.kernel.org
Cc: Eric Wong <e@80x24.org>,
	jrnieder@gmail.com, Brandon Williams <bmwill@google.com>
Subject: [PATCH v5 11/11] run-command: block signals between fork and execve
Date: Tue, 18 Apr 2017 16:18:05 -0700	[thread overview]
Message-ID: <20170418231805.61835-12-bmwill@google.com> (raw)
In-Reply-To: <20170418231805.61835-1-bmwill@google.com>

From: Eric Wong <e@80x24.org>

Signal handlers of the parent firing in the forked child may
have unintended side effects.  Rather than auditing every signal
handler we have and will ever have, block signals while forking
and restore default signal handlers in the child before execve.

Restoring default signal handlers is required because
execve does not unblock signals, it only restores default
signal handlers.  So we must restore them with sigprocmask
before execve, leaving a window when signal handlers
we control can fire in the child.  Continue ignoring
ignored signals, but reset the rest to defaults.

Similarly, disable pthread cancellation to future-proof our code
in case we start using cancellation; as cancellation is
implemented with signals in glibc.

Signed-off-by: Eric Wong <e@80x24.org>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 run-command.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/run-command.c b/run-command.c
index df1edd963..1f3c38e43 100644
--- a/run-command.c
+++ b/run-command.c
@@ -215,6 +215,7 @@ enum child_errcode {
 	CHILD_ERR_CHDIR,
 	CHILD_ERR_DUP2,
 	CHILD_ERR_CLOSE,
+	CHILD_ERR_SIGPROCMASK,
 	CHILD_ERR_ENOENT,
 	CHILD_ERR_SILENT,
 	CHILD_ERR_ERRNO
@@ -303,6 +304,9 @@ static void child_err_spew(struct child_process *cmd, struct child_err *cerr)
 	case CHILD_ERR_CLOSE:
 		error_errno("close() in child failed");
 		break;
+	case CHILD_ERR_SIGPROCMASK:
+		error_errno("sigprocmask failed restoring signals");
+		break;
 	case CHILD_ERR_ENOENT:
 		error_errno("cannot run %s", cmd->argv[0]);
 		break;
@@ -400,6 +404,53 @@ static char **prep_childenv(const char *const *deltaenv)
 }
 #endif
 
+struct atfork_state {
+#ifndef NO_PTHREADS
+	int cs;
+#endif
+	sigset_t old;
+};
+
+#ifndef NO_PTHREADS
+static void bug_die(int err, const char *msg)
+{
+	if (err) {
+		errno = err;
+		die_errno("BUG: %s", msg);
+	}
+}
+#endif
+
+static void atfork_prepare(struct atfork_state *as)
+{
+	sigset_t all;
+
+	if (sigfillset(&all))
+		die_errno("sigfillset");
+#ifdef NO_PTHREADS
+	if (sigprocmask(SIG_SETMASK, &all, &as->old))
+		die_errno("sigprocmask");
+#else
+	bug_die(pthread_sigmask(SIG_SETMASK, &all, &as->old),
+		"blocking all signals");
+	bug_die(pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &as->cs),
+		"disabling cancellation");
+#endif
+}
+
+static void atfork_parent(struct atfork_state *as)
+{
+#ifdef NO_PTHREADS
+	if (sigprocmask(SIG_SETMASK, &as->old, NULL))
+		die_errno("sigprocmask");
+#else
+	bug_die(pthread_setcancelstate(as->cs, NULL),
+		"re-enabling cancellation");
+	bug_die(pthread_sigmask(SIG_SETMASK, &as->old, NULL),
+		"restoring signal mask");
+#endif
+}
+
 static inline void set_cloexec(int fd)
 {
 	int flags = fcntl(fd, F_GETFD);
@@ -523,6 +574,7 @@ int start_command(struct child_process *cmd)
 	char **childenv;
 	struct argv_array argv = ARGV_ARRAY_INIT;
 	struct child_err cerr;
+	struct atfork_state as;
 
 	if (pipe(notify_pipe))
 		notify_pipe[0] = notify_pipe[1] = -1;
@@ -536,6 +588,7 @@ int start_command(struct child_process *cmd)
 
 	prepare_cmd(&argv, cmd);
 	childenv = prep_childenv(cmd->env);
+	atfork_prepare(&as);
 
 	/*
 	 * NOTE: In order to prevent deadlocking when using threads special
@@ -549,6 +602,7 @@ int start_command(struct child_process *cmd)
 	cmd->pid = fork();
 	failed_errno = errno;
 	if (!cmd->pid) {
+		int sig;
 		/*
 		 * Ensure the default die/error/warn routines do not get
 		 * called, they can take stdio locks and malloc.
@@ -597,6 +651,19 @@ int start_command(struct child_process *cmd)
 			child_die(CHILD_ERR_CHDIR);
 
 		/*
+		 * restore default signal handlers here, in case
+		 * we catch a signal right before execve below
+		 */
+		for (sig = 1; sig < NSIG; sig++) {
+			/* ignored signals get reset to SIG_DFL on execve */
+			if (signal(sig, SIG_DFL) == SIG_IGN)
+				signal(sig, SIG_IGN);
+		}
+
+		if (sigprocmask(SIG_SETMASK, &as.old, NULL) != 0)
+			child_die(CHILD_ERR_SIGPROCMASK);
+
+		/*
 		 * Attempt to exec using the command and arguments starting at
 		 * argv.argv[1].  argv.argv[0] contains SHELL_PATH which will
 		 * be used in the event exec failed with ENOEXEC at which point
@@ -616,6 +683,7 @@ int start_command(struct child_process *cmd)
 			child_die(CHILD_ERR_ERRNO);
 		}
 	}
+	atfork_parent(&as);
 	if (cmd->pid < 0)
 		error_errno("cannot fork() for %s", cmd->argv[0]);
 	else if (cmd->clean_on_exit)
-- 
2.12.2.816.g2cccc81164-goog


  parent reply	other threads:[~2017-04-18 23:18 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   ` [PATCH v2 2/6] run-command: prepare command before forking Brandon Williams
2017-04-13 21:14     ` 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         ` Brandon Williams [this message]
2017-04-19  6:00           ` [PATCH v5 11/11] run-command: block signals between fork and execve 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20170418231805.61835-12-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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).