undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

run-command: eliminate calls to error handling functions in child

Browse Source 
All of our standard error handling paths have the potential to
call malloc or take stdio locks; so we must avoid them inside
the forked child.

Instead, the child only writes an 8 byte struct atomically to
the parent through the notification pipe to propagate an error.
All user-visible error reporting happens from the parent;
even avoiding functions like atexit(3) and exit(3).

Helped-by: Eric Wong <e@80x24.org>
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Brandon Williams 2017-04-19 16:13:24 -07:00 committed by Junio C Hamano
parent db015a284e
commit 79319b1949
1 changed files with 89 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

121
run-command.c
View File

@ -211,14 +211,82 @@ static const char **prepare_shell_cmd(struct argv_array *out, const char **argv)
#ifndef GIT_WINDOWS_NATIVE
static int child_notifier = -1;
static void notify_parent(void)
enum child_errcode {
CHILD_ERR_CHDIR,
CHILD_ERR_ENOENT,
CHILD_ERR_SILENT,
CHILD_ERR_ERRNO
};
struct child_err {
enum child_errcode err;
int syserr; /* errno */
};
static void child_die(enum child_errcode err)
{
/*
* execvp failed. If possible, we'd like to let start_command
* know, so failures like ENOENT can be handled right away; but
* otherwise, finish_command will still report the error.
*/
xwrite(child_notifier, "", 1);
struct child_err buf;
buf.err = err;
buf.syserr = errno;
/* write(2) on buf smaller than PIPE_BUF (min 512) is atomic: */
xwrite(child_notifier, &buf, sizeof(buf));
_exit(1);
}
/*
* parent will make it look like the child spewed a fatal error and died
* this is needed to prevent changes to t0061.
*/
static void fake_fatal(const char *err, va_list params)
{
vreportf("fatal: ", err, params);
}
static void child_error_fn(const char *err, va_list params)
{
const char msg[] = "error() should not be called in child\n";
xwrite(2, msg, sizeof(msg) - 1);
}
static void child_warn_fn(const char *err, va_list params)
{
const char msg[] = "warn() should not be called in child\n";
xwrite(2, msg, sizeof(msg) - 1);
}
static void NORETURN child_die_fn(const char *err, va_list params)
{
const char msg[] = "die() should not be called in child\n";
xwrite(2, msg, sizeof(msg) - 1);
_exit(2);
}
/* this runs in the parent process */
static void child_err_spew(struct child_process *cmd, struct child_err *cerr)
{
static void (*old_errfn)(const char *err, va_list params);
old_errfn = get_error_routine();
set_error_routine(fake_fatal);
errno = cerr->syserr;
switch (cerr->err) {
case CHILD_ERR_CHDIR:
error_errno("exec '%s': cd to '%s' failed",
cmd->argv[0], cmd->dir);
break;
case CHILD_ERR_ENOENT:
error_errno("cannot run %s", cmd->argv[0]);
break;
case CHILD_ERR_SILENT:
break;
case CHILD_ERR_ERRNO:
error_errno("cannot exec '%s'", cmd->argv[0]);
break;
}
set_error_routine(old_errfn);
}
static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)
@ -341,13 +409,6 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
code += 128;
} else if (WIFEXITED(status)) {
code = WEXITSTATUS(status);
/*
* Convert special exit code when execvp failed.
*/
if (code == 127) {
code = -1;
failed_errno = ENOENT;
}
} else {
error("waitpid is confused (%s)", argv0);
}
@ -435,6 +496,7 @@ fail_pipe:
int null_fd = -1;
char **childenv;
struct argv_array argv = ARGV_ARRAY_INIT;
struct child_err cerr;
if (pipe(notify_pipe))
notify_pipe[0] = notify_pipe[1] = -1;
@ -453,20 +515,16 @@ fail_pipe:
failed_errno = errno;
if (!cmd->pid) {
/*
* Redirect the channel to write syscall error messages to
* before redirecting the process's stderr so that all die()
* in subsequent call paths use the parent's stderr.
* Ensure the default die/error/warn routines do not get
* called, they can take stdio locks and malloc.
*/
if (cmd->no_stderr || need_err) {
int child_err = dup(2);
set_cloexec(child_err);
set_error_handle(fdopen(child_err, "w"));
}
set_die_routine(child_die_fn);
set_error_routine(child_error_fn);
set_warn_routine(child_warn_fn);
close(notify_pipe[0]);
set_cloexec(notify_pipe[1]);
child_notifier = notify_pipe[1];
atexit(notify_parent);
if (cmd->no_stdin)
dup2(null_fd, 0);
@ -501,8 +559,7 @@ fail_pipe:
}
if (cmd->dir && chdir(cmd->dir))
die_errno("exec '%s': cd to '%s' failed", cmd->argv[0],
cmd->dir);
child_die(CHILD_ERR_CHDIR);
/*
* Attempt to exec using the command and arguments starting at
@ -517,12 +574,11 @@ fail_pipe:
(char *const *) childenv);
if (errno == ENOENT) {
if (!cmd->silent_exec_failure)
error("cannot run %s: %s", cmd->argv[0],
strerror(ENOENT));
exit(127);
if (cmd->silent_exec_failure)
child_die(CHILD_ERR_SILENT);
child_die(CHILD_ERR_ENOENT);
} else {
die_errno("cannot exec '%s'", cmd->argv[0]);
child_die(CHILD_ERR_ERRNO);
}
}
if (cmd->pid < 0)
@ -533,17 +589,18 @@ fail_pipe:
/*
* 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.
* child process sends a child_err struct.
* Note that use of this infrastructure is completely advisory,
* therefore, we keep error checks minimal.
*/
close(notify_pipe[1]);
if (read(notify_pipe[0], &notify_pipe[1], 1) == 1) {
if (xread(notify_pipe[0], &cerr, sizeof(cerr)) == sizeof(cerr)) {
/*
* 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);
child_err_spew(cmd, &cerr);
failed_errno = errno;
cmd->pid = -1;
}