Show 167 quoted lines
>
> -------8<-----
> Subject: [PATCH] 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>
> ---
> Changes from my original in <20170411070534.GA10552@whir>:
>
> - fixed typo in NO_PTHREADS code
>
> - dropped fflush(NULL) before fork, consider us screwed anyways
> if the child uses stdio
>
> - respect SIG_IGN in child; that seems to be the prevailing
> wisdom from reading https://ewontfix.com/7/ and process.c
> in ruby (git clone https://github.com/ruby/ruby.git)
>
> run-command.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/run-command.c b/run-command.c
> index 1c36e692d..59a8b4806 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -213,6 +213,7 @@ static int child_notifier = -1;
>
> enum child_errcode {
> CHILD_ERR_CHDIR,
> + CHILD_ERR_SIGPROCMASK,
> CHILD_ERR_ENOENT,
> CHILD_ERR_SILENT,
> CHILD_ERR_ERRNO,
> @@ -277,6 +278,8 @@ static void child_err_spew(struct child_process *cmd, struct child_err *cerr)
> error_errno("exec '%s': cd to '%s' failed",
> cmd->argv[0], cmd->dir);
> break;
> + case CHILD_ERR_SIGPROCMASK:
> + error_errno("sigprocmask failed restoring signals");
> case CHILD_ERR_ENOENT:
> error_errno("cannot run %s", cmd->argv[0]);
> break;
> @@ -388,6 +391,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);
> @@ -511,6 +561,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;
> @@ -524,6 +575,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
> @@ -537,6 +589,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.
> @@ -584,6 +637,21 @@ int start_command(struct child_process *cmd)
> 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++) {
> + sighandler_t old = signal(sig, SIG_DFL);
> +
> + /* ignored signals get reset to SIG_DFL on execve */
> + if (old == SIG_IGN)
> + signal(sig, SIG_IGN);
> + }
> +
> + if (sigprocmask(SIG_SETMASK, &as.old, NULL) != 0)
> + child_die(CHILD_ERR_SIGPROCMASK);
> +
> execve(argv.argv[0], (char *const *) argv.argv,
> (char *const *) childenv);
>
> @@ -595,6 +663,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)
> --
> EW