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

Merge branch 'bw/forking-and-threading'

Browse Source 
The "run-command" API implementation has been made more robust
against dead-locking in a threaded environment.

* bw/forking-and-threading:
  usage.c: drop set_error_handle()
  run-command: restrict PATH search to executable files
  run-command: expose is_executable function
  run-command: block signals between fork and execve
  run-command: add note about forking and threading
  run-command: handle dup2 and close errors in child
  run-command: eliminate calls to error handling functions in child
  run-command: don't die in child when duping /dev/null
  run-command: prepare child environment before forking
  string-list: add string_list_remove function
  run-command: use the async-signal-safe execv instead of execvp
  run-command: prepare command before forking
  t0061: run_command executes scripts without a #! line
  t5550: use write_script to generate post-update hook
This commit is contained in:
Junio C Hamano 2017-05-30 11:16:41 +09:00
commit 7d5e13f652
9 changed files with 451 additions and 136 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
git-compat-util.h
View File

@ -450,7 +450,6 @@ extern void (*get_error_routine(void))(const char *err, va_list params);
extern void set_warn_routine(void (*routine)(const char *warn, va_list params));
extern void (*get_warn_routine(void))(const char *warn, va_list params);
extern void set_die_is_recursing_routine(int (*routine)(void));
extern void set_error_handle(FILE *);
extern int starts_with(const char *str, const char *prefix);

43
help.c
View File

@ -1,6 +1,7 @@
#include "cache.h"
#include "builtin.h"
#include "exec_cmd.h"
#include "run-command.h"
#include "levenshtein.h"
#include "help.h"
#include "common-cmds.h"
@ -96,48 +97,6 @@ static void pretty_print_cmdnames(struct cmdnames *cmds, unsigned int colopts)
string_list_clear(&list, 0);
}
static int is_executable(const char *name)
{
struct stat st;
if (stat(name, &st) || /* stat, not lstat */
!S_ISREG(st.st_mode))
return 0;
#if defined(GIT_WINDOWS_NATIVE)
/*
* On Windows there is no executable bit. The file extension
* indicates whether it can be run as an executable, and Git
* has special-handling to detect scripts and launch them
* through the indicated script interpreter. We test for the
* file extension first because virus scanners may make
* it quite expensive to open many files.
*/
if (ends_with(name, ".exe"))
return S_IXUSR;
{
/*
* Now that we know it does not have an executable extension,
* peek into the file instead.
*/
char buf[3] = { 0 };
int n;
int fd = open(name, O_RDONLY);
st.st_mode &= ~S_IXUSR;
if (fd >= 0) {
n = read(fd, buf, 2);
if (n == 2)
/* look for a she-bang */
if (!strcmp(buf, "#!"))
st.st_mode |= S_IXUSR;
close(fd);
}
}
#endif
return st.st_mode & S_IXUSR;
}
static void list_commands_in_dir(struct cmdnames *cmds,
const char *path,
const char *prefix)

461
run-command.c
View File

@ -117,18 +117,65 @@ static inline void close_pair(int fd[2])
close(fd[1]);
}
#ifndef GIT_WINDOWS_NATIVE
static inline void dup_devnull(int to)
int is_executable(const char *name)
{
int fd = open("/dev/null", O_RDWR);
if (fd < 0)
die_errno(_("open /dev/null failed"));
if (dup2(fd, to) < 0)
die_errno(_("dup2(%d,%d) failed"), fd, to);
close(fd);
struct stat st;
if (stat(name, &st) || /* stat, not lstat */
!S_ISREG(st.st_mode))
return 0;
#if defined(GIT_WINDOWS_NATIVE)
/*
* On Windows there is no executable bit. The file extension
* indicates whether it can be run as an executable, and Git
* has special-handling to detect scripts and launch them
* through the indicated script interpreter. We test for the
* file extension first because virus scanners may make
* it quite expensive to open many files.
*/
if (ends_with(name, ".exe"))
return S_IXUSR;
{
/*
* Now that we know it does not have an executable extension,
* peek into the file instead.
*/
char buf[3] = { 0 };
int n;
int fd = open(name, O_RDONLY);
st.st_mode &= ~S_IXUSR;
if (fd >= 0) {
n = read(fd, buf, 2);
if (n == 2)
/* look for a she-bang */
if (!strcmp(buf, "#!"))
st.st_mode |= S_IXUSR;
close(fd);
}
}
#endif
return st.st_mode & S_IXUSR;
}
/*
* Search $PATH for a command. This emulates the path search that
* execvp would perform, without actually executing the command so it
* can be used before fork() to prepare to run a command using
* execve() or after execvp() to diagnose why it failed.
*
* The caller should ensure that file contains no directory
* separators.
*
* Returns the path to the command, as found in $PATH or NULL if the
* command could not be found. The caller inherits ownership of the memory
* used to store the resultant path.
*
* This should not be used on Windows, where the $PATH search rules
* are more complicated (e.g., a search for "foo" should find
* "foo.exe").
*/
static char *locate_in_PATH(const char *file)
{
const char *p = getenv("PATH");
@ -149,7 +196,7 @@ static char *locate_in_PATH(const char *file)
}
strbuf_addstr(&buf, file);
if (!access(buf.buf, F_OK))
if (is_executable(buf.buf))
return strbuf_detach(&buf, NULL);
if (!*end)
@ -220,32 +267,249 @@ static const char **prepare_shell_cmd(struct argv_array *out, const char **argv)
return out->argv;
}
#ifndef GIT_WINDOWS_NATIVE
static int execv_shell_cmd(const char **argv)
{
struct argv_array nargv = ARGV_ARRAY_INIT;
prepare_shell_cmd(&nargv, argv);
trace_argv_printf(nargv.argv, "trace: exec:");
sane_execvp(nargv.argv[0], (char **)nargv.argv);
argv_array_clear(&nargv);
return -1;
}
#endif
#ifndef GIT_WINDOWS_NATIVE
static int child_notifier = -1;
static void notify_parent(void)
enum child_errcode {
CHILD_ERR_CHDIR,
CHILD_ERR_DUP2,
CHILD_ERR_CLOSE,
CHILD_ERR_SIGPROCMASK,
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)
{
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);
}
static void child_dup2(int fd, int to)
{
if (dup2(fd, to) < 0)
child_die(CHILD_ERR_DUP2);
}
static void child_close(int fd)
{
if (close(fd))
child_die(CHILD_ERR_CLOSE);
}
static void child_close_pair(int fd[2])
{
child_close(fd[0]);
child_close(fd[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_DUP2:
error_errno("dup2() 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_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)
{
if (!cmd->argv[0])
die("BUG: command is empty");
/*
* 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.
* Add SHELL_PATH so in the event exec fails with ENOEXEC we can
* attempt to interpret the command with 'sh'.
*/
xwrite(child_notifier, "", 1);
argv_array_push(out, SHELL_PATH);
if (cmd->git_cmd) {
argv_array_push(out, "git");
argv_array_pushv(out, cmd->argv);
} else if (cmd->use_shell) {
prepare_shell_cmd(out, cmd->argv);
} else {
argv_array_pushv(out, cmd->argv);
}
/*
* If there are no '/' characters in the command then perform a path
* lookup and use the resolved path as the command to exec. If there
* are no '/' characters or if the command wasn't found in the path,
* have exec attempt to invoke the command directly.
*/
if (!strchr(out->argv[1], '/')) {
char *program = locate_in_PATH(out->argv[1]);
if (program) {
free((char *)out->argv[1]);
out->argv[1] = program;
}
}
}
static char **prep_childenv(const char *const *deltaenv)
{
extern char **environ;
char **childenv;
struct string_list env = STRING_LIST_INIT_DUP;
struct strbuf key = STRBUF_INIT;
const char *const *p;
int i;
/* Construct a sorted string list consisting of the current environ */
for (p = (const char *const *) environ; p && *p; p++) {
const char *equals = strchr(*p, '=');
if (equals) {
strbuf_reset(&key);
strbuf_add(&key, *p, equals - *p);
string_list_append(&env, key.buf)->util = (void *) *p;
} else {
string_list_append(&env, *p)->util = (void *) *p;
}
}
string_list_sort(&env);
/* Merge in 'deltaenv' with the current environ */
for (p = deltaenv; p && *p; p++) {
const char *equals = strchr(*p, '=');
if (equals) {
/* ('key=value'), insert or replace entry */
strbuf_reset(&key);
strbuf_add(&key, *p, equals - *p);
string_list_insert(&env, key.buf)->util = (void *) *p;
} else {
/* otherwise ('key') remove existing entry */
string_list_remove(&env, *p, 0);
}
}
/* Create an array of 'char *' to be used as the childenv */
childenv = xmalloc((env.nr + 1) * sizeof(char *));
for (i = 0; i < env.nr; i++)
childenv[i] = env.items[i].util;
childenv[env.nr] = NULL;
string_list_clear(&env, 0);
strbuf_release(&key);
return childenv;
}
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
}
#endif /* GIT_WINDOWS_NATIVE */
static inline void set_cloexec(int fd)
{
int flags = fcntl(fd, F_GETFD);
@ -281,13 +545,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);
}
@ -372,109 +629,149 @@ fail_pipe:
#ifndef GIT_WINDOWS_NATIVE
{
int notify_pipe[2];
int null_fd = -1;
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 (cmd->no_stdin || cmd->no_stdout || cmd->no_stderr) {
null_fd = open("/dev/null", O_RDWR | O_CLOEXEC);
if (null_fd < 0)
die_errno(_("open /dev/null failed"));
set_cloexec(null_fd);
}
prepare_cmd(&argv, cmd);
childenv = prep_childenv(cmd->env);
atfork_prepare(&as);
/*
* NOTE: In order to prevent deadlocking when using threads special
* care should be taken with the function calls made in between the
* fork() and exec() calls. No calls should be made to functions which
* require acquiring a lock (e.g. malloc) as the lock could have been
* held by another thread at the time of forking, causing the lock to
* never be released in the child process. This means only
* Async-Signal-Safe functions are permitted in the child.
*/
cmd->pid = fork();
failed_errno = errno;
if (!cmd->pid) {
int sig;
/*
* 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)
dup_devnull(0);
child_dup2(null_fd, 0);
else if (need_in) {
dup2(fdin[0], 0);
close_pair(fdin);
child_dup2(fdin[0], 0);
child_close_pair(fdin);
} else if (cmd->in) {
dup2(cmd->in, 0);
close(cmd->in);
child_dup2(cmd->in, 0);
child_close(cmd->in);
}
if (cmd->no_stderr)
dup_devnull(2);
child_dup2(null_fd, 2);
else if (need_err) {
dup2(fderr[1], 2);
close_pair(fderr);
child_dup2(fderr[1], 2);
child_close_pair(fderr);
} else if (cmd->err > 1) {
dup2(cmd->err, 2);
close(cmd->err);
child_dup2(cmd->err, 2);
child_close(cmd->err);
}
if (cmd->no_stdout)
dup_devnull(1);
child_dup2(null_fd, 1);
else if (cmd->stdout_to_stderr)
dup2(2, 1);
child_dup2(2, 1);
else if (need_out) {
dup2(fdout[1], 1);
close_pair(fdout);
child_dup2(fdout[1], 1);
child_close_pair(fdout);
} else if (cmd->out > 1) {
dup2(cmd->out, 1);
close(cmd->out);
child_dup2(cmd->out, 1);
child_close(cmd->out);
}
if (cmd->dir && chdir(cmd->dir))
die_errno("exec '%s': cd to '%s' failed", cmd->argv[0],
cmd->dir);
if (cmd->env) {
for (; *cmd->env; cmd->env++) {
if (strchr(*cmd->env, '='))
putenv((char *)*cmd->env);
else
unsetenv(*cmd->env);
}
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 (cmd->git_cmd)
execv_git_cmd(cmd->argv);
else if (cmd->use_shell)
execv_shell_cmd(cmd->argv);
else
sane_execvp(cmd->argv[0], (char *const*) cmd->argv);
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
* we will try to interpret the command using 'sh'.
*/
execve(argv.argv[1], (char *const *) argv.argv + 1,
(char *const *) childenv);
if (errno == ENOEXEC)
execve(argv.argv[0], (char *const *) argv.argv,
(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);
}
}
atfork_parent(&as);
if (cmd->pid < 0)
error_errno("cannot fork() for %s", cmd->argv[0]);
else if (cmd->clean_on_exit)
mark_child_for_cleanup(cmd->pid, cmd);
/*
* Wait for child's execvp. If the execvp succeeds (or if fork()
* 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 execvp()
* 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;
}
close(notify_pipe[0]);
if (null_fd >= 0)
close(null_fd);
argv_array_clear(&argv);
free(childenv);
}
#else
{

1
run-command.h
View File

@ -51,6 +51,7 @@ struct child_process {
#define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }
void child_process_init(struct child_process *);
void child_process_clear(struct child_process *);
extern int is_executable(const char *name);
int start_command(struct child_process *);
int finish_command(struct child_process *);

18
string-list.c
View File

@ -64,6 +64,24 @@ struct string_list_item *string_list_insert(struct string_list *list, const char
return list->items + index;
}
void string_list_remove(struct string_list *list, const char *string,
int free_util)
{
int exact_match;
int i = get_entry_index(list, string, &exact_match);
if (exact_match) {
if (list->strdup_strings)
free(list->items[i].string);
if (free_util)
free(list->items[i].util);
list->nr--;
memmove(list->items + i, list->items + i + 1,
(list->nr - i) * sizeof(struct string_list_item));
}
}
int string_list_has_string(const struct string_list *list, const char *string)
{
int exact_match;

7
string-list.h
View File

@ -62,6 +62,13 @@ int string_list_find_insert_index(const struct string_list *list, const char *st
*/
struct string_list_item *string_list_insert(struct string_list *list, const char *string);
/*
* Removes the given string from the sorted list.
* If the string doesn't exist, the list is not altered.
*/
extern void string_list_remove(struct string_list *list, const char *string,
int free_util);
/*
* Checks if the given string is part of a sorted list. If it is part of the list,
* return the coresponding string_list_item, NULL otherwise.

41
t/t0061-run-command.sh
View File

@ -26,6 +26,47 @@ test_expect_success 'run_command can run a command' '
test_cmp empty err
'
test_expect_success !MINGW 'run_command can run a script without a #! line' '
cat >hello <<-\EOF &&
cat hello-script
EOF
chmod +x hello &&
test-run-command run-command ./hello >actual 2>err &&
test_cmp hello-script actual &&
test_cmp empty err
'
test_expect_success 'run_command does not try to execute a directory' '
test_when_finished "rm -rf bin1 bin2" &&
mkdir -p bin1/greet bin2 &&
write_script bin2/greet <<-\EOF &&
cat bin2/greet
EOF
PATH=$PWD/bin1:$PWD/bin2:$PATH \
test-run-command run-command greet >actual 2>err &&
test_cmp bin2/greet actual &&
test_cmp empty err
'
test_expect_success POSIXPERM 'run_command passes over non-executable file' '
test_when_finished "rm -rf bin1 bin2" &&
mkdir -p bin1 bin2 &&
write_script bin1/greet <<-\EOF &&
cat bin1/greet
EOF
chmod -x bin1/greet &&
write_script bin2/greet <<-\EOF &&
cat bin2/greet
EOF
PATH=$PWD/bin1:$PWD/bin2:$PATH \
test-run-command run-command greet >actual 2>err &&
test_cmp bin2/greet actual &&
test_cmp empty err
'
test_expect_success POSIXPERM 'run_command reports EACCES' '
cat hello-script >hello.sh &&
chmod -x hello.sh &&

5
t/t5550-http-fetch-dumb.sh
View File

@ -20,8 +20,9 @@ test_expect_success 'create http-accessible bare repository with loose objects'
(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
git config core.bare true &&
mkdir -p hooks &&
echo "exec git update-server-info" >hooks/post-update &&
chmod +x hooks/post-update &&
write_script "hooks/post-update" <<-\EOF &&
exec git update-server-info
EOF
hooks/post-update
) &&
git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&

10
usage.c
View File

@ -6,12 +6,9 @@
#include "git-compat-util.h"
#include "cache.h"
static FILE *error_handle;
void vreportf(const char *prefix, const char *err, va_list params)
{
char msg[4096];
FILE *fh = error_handle ? error_handle : stderr;
char *p;
vsnprintf(msg, sizeof(msg), err, params);
@ -19,7 +16,7 @@ void vreportf(const char *prefix, const char *err, va_list params)
if (iscntrl(*p) && *p != '\t' && *p != '\n')
*p = '?';
}
fprintf(fh, "%s%s\n", prefix, msg);
fprintf(stderr, "%s%s\n", prefix, msg);
}
static NORETURN void usage_builtin(const char *err, va_list params)
@ -88,11 +85,6 @@ void set_die_is_recursing_routine(int (*routine)(void))
die_is_recursing = routine;
}
void set_error_handle(FILE *fh)
{
error_handle = fh;
}
void NORETURN usagef(const char *err, ...)
{
va_list params;