run-command: block signals between fork and execve

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>

Eric Wong committed Apr 19, 2017 at 16:13 UTC 45afb1ca9c28855096c94926e5b16dfbcde7381f
1 file changed +68
run-command.c
+68
@@ -215,6 +215,7 @@ enum child_errcode {
215 CHILD_ERR_CHDIR,
216 CHILD_ERR_DUP2,
217 CHILD_ERR_CLOSE,
218 + CHILD_ERR_SIGPROCMASK,
219 CHILD_ERR_ENOENT,
220 CHILD_ERR_SILENT,
221 CHILD_ERR_ERRNO
@@ -303,6 +304,9 @@ static void child_err_spew(struct child_process *cmd, struct child_err *cerr)
304 case CHILD_ERR_CLOSE:
305 error_errno("close() in child failed");
306 break;
307 + case CHILD_ERR_SIGPROCMASK:
308 + error_errno("sigprocmask failed restoring signals");
309 + break;
310 case CHILD_ERR_ENOENT:
311 error_errno("cannot run %s", cmd->argv[0]);
312 break;
@@ -398,7 +402,54 @@ static char **prep_childenv(const char *const *deltaenv)
402 strbuf_release(&key);
403 return childenv;
404 }
405 +
406 +struct atfork_state {
407 +#ifndef NO_PTHREADS
408 + int cs;
409 #endif
410 + sigset_t old;
411 +};
412 +
413 +#ifndef NO_PTHREADS
414 +static void bug_die(int err, const char *msg)
415 +{
416 + if (err) {
417 + errno = err;
418 + die_errno("BUG: %s", msg);
419 + }
420 +}
421 +#endif
422 +
423 +static void atfork_prepare(struct atfork_state *as)
424 +{
425 + sigset_t all;
426 +
427 + if (sigfillset(&all))
428 + die_errno("sigfillset");
429 +#ifdef NO_PTHREADS
430 + if (sigprocmask(SIG_SETMASK, &all, &as->old))
431 + die_errno("sigprocmask");
432 +#else
433 + bug_die(pthread_sigmask(SIG_SETMASK, &all, &as->old),
434 + "blocking all signals");
435 + bug_die(pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &as->cs),
436 + "disabling cancellation");
437 +#endif
438 +}
439 +
440 +static void atfork_parent(struct atfork_state *as)
441 +{
442 +#ifdef NO_PTHREADS
443 + if (sigprocmask(SIG_SETMASK, &as->old, NULL))
444 + die_errno("sigprocmask");
445 +#else
446 + bug_die(pthread_setcancelstate(as->cs, NULL),
447 + "re-enabling cancellation");
448 + bug_die(pthread_sigmask(SIG_SETMASK, &as->old, NULL),
449 + "restoring signal mask");
450 +#endif
451 +}
452 +#endif /* GIT_WINDOWS_NATIVE */
453
454 static inline void set_cloexec(int fd)
455 {
@@ -523,6 +574,7 @@ fail_pipe:
574 char **childenv;
575 struct argv_array argv = ARGV_ARRAY_INIT;
576 struct child_err cerr;
577 + struct atfork_state as;
578
579 if (pipe(notify_pipe))
580 notify_pipe[0] = notify_pipe[1] = -1;
@@ -536,6 +588,7 @@ fail_pipe:
588
589 prepare_cmd(&argv, cmd);
590 childenv = prep_childenv(cmd->env);
591 + atfork_prepare(&as);
592
593 /*
594 * NOTE: In order to prevent deadlocking when using threads special
@@ -549,6 +602,7 @@ fail_pipe:
602 cmd->pid = fork();
603 failed_errno = errno;
604 if (!cmd->pid) {
605 + int sig;
606 /*
607 * Ensure the default die/error/warn routines do not get
608 * called, they can take stdio locks and malloc.
@@ -596,6 +650,19 @@ fail_pipe:
650 if (cmd->dir && chdir(cmd->dir))
651 child_die(CHILD_ERR_CHDIR);
652
653 + /*
654 + * restore default signal handlers here, in case
655 + * we catch a signal right before execve below
656 + */
657 + for (sig = 1; sig < NSIG; sig++) {
658 + /* ignored signals get reset to SIG_DFL on execve */
659 + if (signal(sig, SIG_DFL) == SIG_IGN)
660 + signal(sig, SIG_IGN);
661 + }
662 +
663 + if (sigprocmask(SIG_SETMASK, &as.old, NULL) != 0)
664 + child_die(CHILD_ERR_SIGPROCMASK);
665 +
666 /*
667 * Attempt to exec using the command and arguments starting at
668 * argv.argv[1]. argv.argv[0] contains SHELL_PATH which will
@@ -616,6 +683,7 @@ fail_pipe:
683 child_die(CHILD_ERR_ERRNO);
684 }
685 }
686 + atfork_parent(&as);
687 if (cmd->pid < 0)
688 error_errno("cannot fork() for %s", cmd->argv[0]);
689 else if (cmd->clean_on_exit)