Merge branch 'js/mingw-inherit-only-std-handles'

Work around a issue where a FD that is left open when spawning a
child process and is kept open in the child can interfere with the
operation in the parent process on Windows.

* js/mingw-inherit-only-std-handles:
  mingw: forbid translating ERROR_SUCCESS to an errno value
  mingw: do set `errno` correctly when trying to restrict handle inheritance
  mingw: restrict file handle inheritance only on Windows 7 and later
  mingw: spawned processes need to inherit only standard handles
  mingw: work around incorrect standard handles
  mingw: demonstrate that all file handles are inherited by child processes
This commit is contained in:
Junio C Hamano 2019-12-10 13:11:42 -08:00
Родитель 7c88714262 3ba3720b3f
Коммит 55d607d85b
5 изменённых файлов: 199 добавлений и 12 удалений

Просмотреть файл

@ -559,6 +559,12 @@ core.unsetenvvars::
Defaults to `PERL5LIB` to account for the fact that Git for
Windows insists on using its own Perl interpreter.
core.restrictinheritedhandles::
Windows-only: override whether spawned processes inherit only standard
file handles (`stdin`, `stdout` and `stderr`) or all handles. Can be
`auto`, `true` or `false`. Defaults to `auto`, which means `true` on
Windows 7 and later, and `false` on older Windows versions.
core.createObject::
You can set this to 'link', in which case a hardlink followed by
a delete of the source are used to make sure that object creation

Просмотреть файл

@ -114,6 +114,7 @@ int err_win_to_posix(DWORD winerr)
case ERROR_SHARING_BUFFER_EXCEEDED: error = ENFILE; break;
case ERROR_SHARING_VIOLATION: error = EACCES; break;
case ERROR_STACK_OVERFLOW: error = ENOMEM; break;
case ERROR_SUCCESS: BUG("err_win_to_posix() called without an error!");
case ERROR_SWAPERROR: error = ENOENT; break;
case ERROR_TOO_MANY_MODULES: error = EMFILE; break;
case ERROR_TOO_MANY_OPEN_FILES: error = EMFILE; break;
@ -212,6 +213,7 @@ enum hide_dotfiles_type {
HIDE_DOTFILES_DOTGITONLY
};
static int core_restrict_inherited_handles = -1;
static enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
static char *unset_environment_variables;
@ -231,6 +233,15 @@ int mingw_core_config(const char *var, const char *value, void *cb)
return 0;
}
if (!strcmp(var, "core.restrictinheritedhandles")) {
if (value && !strcasecmp(value, "auto"))
core_restrict_inherited_handles = -1;
else
core_restrict_inherited_handles =
git_config_bool(var, value);
return 0;
}
return 0;
}
@ -1436,8 +1447,13 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
const char *dir,
int prepend_cmd, int fhin, int fhout, int fherr)
{
STARTUPINFOW si;
static int restrict_handle_inheritance = -1;
STARTUPINFOEXW si;
PROCESS_INFORMATION pi;
LPPROC_THREAD_ATTRIBUTE_LIST attr_list = NULL;
HANDLE stdhandles[3];
DWORD stdhandles_count = 0;
SIZE_T size;
struct strbuf args;
wchar_t wcmd[MAX_PATH], wdir[MAX_PATH], *wargs, *wenvblk = NULL;
unsigned flags = CREATE_UNICODE_ENVIRONMENT;
@ -1447,6 +1463,19 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
is_msys2_sh(cmd ? cmd : *argv) ?
quote_arg_msys2 : quote_arg_msvc;
/* Make sure to override previous errors, if any */
errno = 0;
if (restrict_handle_inheritance < 0)
restrict_handle_inheritance = core_restrict_inherited_handles;
/*
* The following code to restrict which handles are inherited seems
* to work properly only on Windows 7 and later, so let's disable it
* on Windows Vista and 2008.
*/
if (restrict_handle_inheritance < 0)
restrict_handle_inheritance = GetVersion() >> 16 >= 7601;
do_unset_environment_variables();
/* Determine whether or not we are associated to a console */
@ -1474,11 +1503,23 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
CloseHandle(cons);
}
memset(&si, 0, sizeof(si));
si.cb = sizeof(si);
si.dwFlags = STARTF_USESTDHANDLES;
si.hStdInput = winansi_get_osfhandle(fhin);
si.hStdOutput = winansi_get_osfhandle(fhout);
si.hStdError = winansi_get_osfhandle(fherr);
si.StartupInfo.cb = sizeof(si);
si.StartupInfo.hStdInput = winansi_get_osfhandle(fhin);
si.StartupInfo.hStdOutput = winansi_get_osfhandle(fhout);
si.StartupInfo.hStdError = winansi_get_osfhandle(fherr);
/* The list of handles cannot contain duplicates */
if (si.StartupInfo.hStdInput != INVALID_HANDLE_VALUE)
stdhandles[stdhandles_count++] = si.StartupInfo.hStdInput;
if (si.StartupInfo.hStdOutput != INVALID_HANDLE_VALUE &&
si.StartupInfo.hStdOutput != si.StartupInfo.hStdInput)
stdhandles[stdhandles_count++] = si.StartupInfo.hStdOutput;
if (si.StartupInfo.hStdError != INVALID_HANDLE_VALUE &&
si.StartupInfo.hStdError != si.StartupInfo.hStdInput &&
si.StartupInfo.hStdError != si.StartupInfo.hStdOutput)
stdhandles[stdhandles_count++] = si.StartupInfo.hStdError;
if (stdhandles_count)
si.StartupInfo.dwFlags |= STARTF_USESTDHANDLES;
if (*argv && !strcmp(cmd, *argv))
wcmd[0] = L'\0';
@ -1511,16 +1552,98 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
wenvblk = make_environment_block(deltaenv);
memset(&pi, 0, sizeof(pi));
ret = CreateProcessW(*wcmd ? wcmd : NULL, wargs, NULL, NULL, TRUE,
flags, wenvblk, dir ? wdir : NULL, &si, &pi);
if (restrict_handle_inheritance && stdhandles_count &&
(InitializeProcThreadAttributeList(NULL, 1, 0, &size) ||
GetLastError() == ERROR_INSUFFICIENT_BUFFER) &&
(attr_list = (LPPROC_THREAD_ATTRIBUTE_LIST)
(HeapAlloc(GetProcessHeap(), 0, size))) &&
InitializeProcThreadAttributeList(attr_list, 1, 0, &size) &&
UpdateProcThreadAttribute(attr_list, 0,
PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
stdhandles,
stdhandles_count * sizeof(HANDLE),
NULL, NULL)) {
si.lpAttributeList = attr_list;
flags |= EXTENDED_STARTUPINFO_PRESENT;
}
ret = CreateProcessW(*wcmd ? wcmd : NULL, wargs, NULL, NULL,
stdhandles_count ? TRUE : FALSE,
flags, wenvblk, dir ? wdir : NULL,
&si.StartupInfo, &pi);
/*
* On Windows 2008 R2, it seems that specifying certain types of handles
* (such as FILE_TYPE_CHAR or FILE_TYPE_PIPE) will always produce an
* error. Rather than playing finicky and fragile games, let's just try
* to detect this situation and simply try again without restricting any
* handle inheritance. This is still better than failing to create
* processes.
*/
if (!ret && restrict_handle_inheritance && stdhandles_count) {
DWORD err = GetLastError();
struct strbuf buf = STRBUF_INIT;
if (err != ERROR_NO_SYSTEM_RESOURCES &&
/*
* On Windows 7 and earlier, handles on pipes and character
* devices are inherited automatically, and cannot be
* specified in the thread handle list. Rather than trying
* to catch each and every corner case (and running the
* chance of *still* forgetting a few), let's just fall
* back to creating the process without trying to limit the
* handle inheritance.
*/
!(err == ERROR_INVALID_PARAMETER &&
GetVersion() >> 16 < 9200) &&
!getenv("SUPPRESS_HANDLE_INHERITANCE_WARNING")) {
DWORD fl = 0;
int i;
setenv("SUPPRESS_HANDLE_INHERITANCE_WARNING", "1", 1);
for (i = 0; i < stdhandles_count; i++) {
HANDLE h = stdhandles[i];
strbuf_addf(&buf, "handle #%d: %p (type %lx, "
"handle info (%d) %lx\n", i, h,
GetFileType(h),
GetHandleInformation(h, &fl),
fl);
}
strbuf_addstr(&buf, "\nThis is a bug; please report it "
"at\nhttps://github.com/git-for-windows/"
"git/issues/new\n\n"
"To suppress this warning, please set "
"the environment variable\n\n"
"\tSUPPRESS_HANDLE_INHERITANCE_WARNING=1"
"\n");
}
restrict_handle_inheritance = 0;
flags &= ~EXTENDED_STARTUPINFO_PRESENT;
ret = CreateProcessW(*wcmd ? wcmd : NULL, wargs, NULL, NULL,
TRUE, flags, wenvblk, dir ? wdir : NULL,
&si.StartupInfo, &pi);
if (!ret)
errno = err_win_to_posix(GetLastError());
if (ret && buf.len) {
warning("failed to restrict file handles (%ld)\n\n%s",
err, buf.buf);
}
strbuf_release(&buf);
} else if (!ret)
errno = err_win_to_posix(GetLastError());
if (si.lpAttributeList)
DeleteProcThreadAttributeList(si.lpAttributeList);
if (attr_list)
HeapFree(GetProcessHeap(), 0, attr_list);
free(wenvblk);
free(wargs);
if (!ret) {
errno = ENOENT;
if (!ret)
return -1;
}
CloseHandle(pi.hThread);
/*

Просмотреть файл

@ -662,10 +662,20 @@ void winansi_init(void)
*/
HANDLE winansi_get_osfhandle(int fd)
{
HANDLE ret;
if (fd == 1 && (fd_is_interactive[1] & FD_SWAPPED))
return hconsole1;
if (fd == 2 && (fd_is_interactive[2] & FD_SWAPPED))
return hconsole2;
return (HANDLE)_get_osfhandle(fd);
ret = (HANDLE)_get_osfhandle(fd);
/*
* There are obviously circumstances under which _get_osfhandle()
* returns (HANDLE)-2. This is not documented anywhere, but that is so
* clearly an invalid handle value that we can just work around this
* and return the correct value for invalid handles.
*/
return ret == (HANDLE)-2 ? INVALID_HANDLE_VALUE : ret;
}

Просмотреть файл

@ -328,6 +328,46 @@ static int quote_echo(int argc, const char **argv)
return 0;
}
static int inherit_handle(const char *argv0)
{
struct child_process cp = CHILD_PROCESS_INIT;
char path[PATH_MAX];
int tmp;
/* First, open an inheritable handle */
xsnprintf(path, sizeof(path), "out-XXXXXX");
tmp = xmkstemp(path);
argv_array_pushl(&cp.args,
"test-tool", argv0, "inherited-handle-child", NULL);
cp.in = -1;
cp.no_stdout = cp.no_stderr = 1;
if (start_command(&cp) < 0)
die("Could not start child process");
/* Then close it, and try to delete it. */
close(tmp);
if (unlink(path))
die("Could not delete '%s'", path);
if (close(cp.in) < 0 || finish_command(&cp) < 0)
die("Child did not finish");
return 0;
}
static int inherit_handle_child(void)
{
struct strbuf buf = STRBUF_INIT;
if (strbuf_read(&buf, 0, 0) < 0)
die("Could not read stdin");
printf("Received %s\n", buf.buf);
strbuf_release(&buf);
return 0;
}
int cmd__run_command(int argc, const char **argv)
{
struct child_process proc = CHILD_PROCESS_INIT;
@ -335,6 +375,10 @@ int cmd__run_command(int argc, const char **argv)
if (argc > 1 && !strcmp(argv[1], "testsuite"))
exit(testsuite(argc - 1, argv + 1));
if (!strcmp(argv[1], "inherited-handle"))
exit(inherit_handle(argv[0]));
if (!strcmp(argv[1], "inherited-handle-child"))
exit(inherit_handle_child());
if (argc >= 2 && !strcmp(argv[1], "quote-stress-test"))
return !!quote_stress_test(argc - 1, argv + 1);

Просмотреть файл

@ -12,6 +12,10 @@ cat >hello-script <<-EOF
cat hello-script
EOF
test_expect_success MINGW 'subprocess inherits only std handles' '
test-tool run-command inherited-handle
'
test_expect_success 'start_command reports ENOENT (slash)' '
test-tool run-command start-command-ENOENT ./does-not-exist 2>err &&
test_i18ngrep "\./does-not-exist" err