run-command: block signals between fork and execve
authorEric Wong <e@80x24.org>
Wed, 19 Apr 2017 23:13:27 +0000 (16:13 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 21 Apr 2017 00:55:32 +0000 (17:55 -0700)
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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
run-command.c
index df1edd963f2567486b8d7bbb2926e0320f8dac71..a97d7bf9f35328563675f9d71b1d81854ca03e9d 100644 (file)
@@ -215,6 +215,7 @@ enum child_errcode {
        CHILD_ERR_CHDIR,
        CHILD_ERR_DUP2,
        CHILD_ERR_CLOSE,
        CHILD_ERR_CHDIR,
        CHILD_ERR_DUP2,
        CHILD_ERR_CLOSE,
+       CHILD_ERR_SIGPROCMASK,
        CHILD_ERR_ENOENT,
        CHILD_ERR_SILENT,
        CHILD_ERR_ERRNO
        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_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;
        case CHILD_ERR_ENOENT:
                error_errno("cannot run %s", cmd->argv[0]);
                break;
@@ -398,7 +402,54 @@ static char **prep_childenv(const char *const *deltaenv)
        strbuf_release(&key);
        return childenv;
 }
        strbuf_release(&key);
        return childenv;
 }
+
+struct atfork_state {
+#ifndef NO_PTHREADS
+       int cs;
 #endif
 #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
+}
+#endif /* GIT_WINDOWS_NATIVE */
 
 static inline void set_cloexec(int fd)
 {
 
 static inline void set_cloexec(int fd)
 {
@@ -523,6 +574,7 @@ int start_command(struct child_process *cmd)
        char **childenv;
        struct argv_array argv = ARGV_ARRAY_INIT;
        struct child_err cerr;
        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;
 
        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);
 
        prepare_cmd(&argv, cmd);
        childenv = prep_childenv(cmd->env);
+       atfork_prepare(&as);
 
        /*
         * NOTE: In order to prevent deadlocking when using threads special
 
        /*
         * 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) {
        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.
                /*
                 * Ensure the default die/error/warn routines do not get
                 * called, they can take stdio locks and malloc.
@@ -596,6 +650,19 @@ int start_command(struct child_process *cmd)
                if (cmd->dir && chdir(cmd->dir))
                        child_die(CHILD_ERR_CHDIR);
 
                if (cmd->dir && chdir(cmd->dir))
                        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
                /*
                 * Attempt to exec using the command and arguments starting at
                 * argv.argv[1].  argv.argv[0] contains SHELL_PATH which will
@@ -616,6 +683,7 @@ int start_command(struct child_process *cmd)
                        child_die(CHILD_ERR_ERRNO);
                }
        }
                        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)
        if (cmd->pid < 0)
                error_errno("cannot fork() for %s", cmd->argv[0]);
        else if (cmd->clean_on_exit)