Skip to content

Commit 71beb92

Browse files
dschodscho
committed
Merge branch 'inherit-only-stdhandles'
When spawning child processes, we do want them to inherit the standard handles so that we can talk to them. We do *not* want them to inherit any other handle, as that would hold a lock to the respective files (preventing them from being renamed, modified or deleted), and the child process would not know how to access that handle anyway. Happily, there is an API to make that happen. It is supported in Windows Vista and later, which is exactly what we promise to support in Git for Windows for the time being. This also means that we lift, at long last, the target Windows version from Windows XP to Windows Vista. Signed-off-by: Johannes Schindelin <[email protected]>
2 parents 0db9119 + 6bbe29b commit 71beb92

File tree

4 files changed

+171
-12
lines changed

4 files changed

+171
-12
lines changed

‎compat/mingw.c‎

Lines changed: 109 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,8 +1577,13 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
15771577
const char *dir,
15781578
int prepend_cmd, int fhin, int fhout, int fherr)
15791579
{
1580-
STARTUPINFOW si;
1580+
static int restrict_handle_inheritance = 1;
1581+
STARTUPINFOEXW si;
15811582
PROCESS_INFORMATION pi;
1583+
LPPROC_THREAD_ATTRIBUTE_LIST attr_list = NULL;
1584+
HANDLE stdhandles[3];
1585+
DWORD stdhandles_count = 0;
1586+
SIZE_T size;
15821587
struct strbuf args;
15831588
wchar_t wcmd[MAX_PATH], wdir[MAX_PATH], *wargs, *wenvblk = NULL;
15841589
unsigned flags = CREATE_UNICODE_ENVIRONMENT;
@@ -1615,11 +1620,23 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
16151620
CloseHandle(cons);
16161621
}
16171622
memset(&si, 0, sizeof(si));
1618-
si.cb = sizeof(si);
1619-
si.dwFlags = STARTF_USESTDHANDLES;
1620-
si.hStdInput = winansi_get_osfhandle(fhin);
1621-
si.hStdOutput = winansi_get_osfhandle(fhout);
1622-
si.hStdError = winansi_get_osfhandle(fherr);
1623+
si.StartupInfo.cb = sizeof(si);
1624+
si.StartupInfo.hStdInput = winansi_get_osfhandle(fhin);
1625+
si.StartupInfo.hStdOutput = winansi_get_osfhandle(fhout);
1626+
si.StartupInfo.hStdError = winansi_get_osfhandle(fherr);
1627+
1628+
/* The list of handles cannot contain duplicates */
1629+
if (si.StartupInfo.hStdInput != INVALID_HANDLE_VALUE)
1630+
stdhandles[stdhandles_count++] = si.StartupInfo.hStdInput;
1631+
if (si.StartupInfo.hStdOutput != INVALID_HANDLE_VALUE &&
1632+
si.StartupInfo.hStdOutput != si.StartupInfo.hStdInput)
1633+
stdhandles[stdhandles_count++] = si.StartupInfo.hStdOutput;
1634+
if (si.StartupInfo.hStdError != INVALID_HANDLE_VALUE &&
1635+
si.StartupInfo.hStdError != si.StartupInfo.hStdInput &&
1636+
si.StartupInfo.hStdError != si.StartupInfo.hStdOutput)
1637+
stdhandles[stdhandles_count++] = si.StartupInfo.hStdError;
1638+
if (stdhandles_count)
1639+
si.StartupInfo.dwFlags |= STARTF_USESTDHANDLES;
16231640

16241641
/* executables and the current directory don't support long paths */
16251642
if (*argv && !strcmp(cmd, *argv))
@@ -1678,16 +1695,97 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
16781695
wenvblk = make_environment_block(deltaenv);
16791696

16801697
memset(&pi, 0, sizeof(pi));
1681-
ret = CreateProcessW(*wcmd ? wcmd : NULL, wargs, NULL, NULL, TRUE,
1682-
flags, wenvblk, dir ? wdir : NULL, &si, &pi);
1698+
if (restrict_handle_inheritance && stdhandles_count &&
1699+
(InitializeProcThreadAttributeList(NULL, 1, 0, &size) ||
1700+
GetLastError() == ERROR_INSUFFICIENT_BUFFER) &&
1701+
(attr_list = (LPPROC_THREAD_ATTRIBUTE_LIST)
1702+
(HeapAlloc(GetProcessHeap(), 0, size))) &&
1703+
InitializeProcThreadAttributeList(attr_list, 1, 0, &size) &&
1704+
UpdateProcThreadAttribute(attr_list, 0,
1705+
PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
1706+
stdhandles,
1707+
stdhandles_count * sizeof(HANDLE),
1708+
NULL, NULL)) {
1709+
si.lpAttributeList = attr_list;
1710+
flags |= EXTENDED_STARTUPINFO_PRESENT;
1711+
}
1712+
1713+
ret = CreateProcessW(*wcmd ? wcmd : NULL, wargs, NULL, NULL,
1714+
stdhandles_count ? TRUE : FALSE,
1715+
flags, wenvblk, dir ? wdir : NULL,
1716+
&si.StartupInfo, &pi);
1717+
1718+
/*
1719+
* On Windows 2008 R2, it seems that specifying certain types of handles
1720+
* (such as FILE_TYPE_CHAR or FILE_TYPE_PIPE) will always produce an
1721+
* error. Rather than playing finicky and fragile games, let's just try
1722+
* to detect this situation and simply try again without restricting any
1723+
* handle inheritance. This is still better than failing to create
1724+
* processes.
1725+
*/
1726+
if (!ret && restrict_handle_inheritance && stdhandles_count) {
1727+
DWORD err = GetLastError();
1728+
struct strbuf buf = STRBUF_INIT;
1729+
1730+
if (err != ERROR_NO_SYSTEM_RESOURCES &&
1731+
/*
1732+
* On Windows 7 and earlier, handles on pipes and character
1733+
* devices are inherited automatically, and cannot be
1734+
* specified in the thread handle list. Rather than trying
1735+
* to catch each and every corner case (and running the
1736+
* chance of *still* forgetting a few), let's just fall
1737+
* back to creating the process without trying to limit the
1738+
* handle inheritance.
1739+
*/
1740+
!(err == ERROR_INVALID_PARAMETER &&
1741+
GetVersion() >> 16 < 9200) &&
1742+
!getenv("SUPPRESS_HANDLE_INHERITANCE_WARNING")) {
1743+
DWORD fl = 0;
1744+
int i;
1745+
1746+
setenv("SUPPRESS_HANDLE_INHERITANCE_WARNING", "1", 1);
1747+
1748+
for (i = 0; i < stdhandles_count; i++) {
1749+
HANDLE h = stdhandles[i];
1750+
strbuf_addf(&buf, "handle #%d: %p (type %lx, "
1751+
"handle info (%d) %lx\n", i, h,
1752+
GetFileType(h),
1753+
GetHandleInformation(h, &fl),
1754+
fl);
1755+
}
1756+
strbuf_addstr(&buf, "\nThis is a bug; please report it "
1757+
"at\nhttps://github.com/git-for-windows/"
1758+
"git/issues/new\n\n"
1759+
"To suppress this warning, please set "
1760+
"the environment variable\n\n"
1761+
"\tSUPPRESS_HANDLE_INHERITANCE_WARNING=1"
1762+
"\n");
1763+
}
1764+
restrict_handle_inheritance = 0;
1765+
flags &= ~EXTENDED_STARTUPINFO_PRESENT;
1766+
ret = CreateProcessW(*wcmd ? wcmd : NULL, wargs, NULL, NULL,
1767+
TRUE, flags, wenvblk, dir ? wdir : NULL,
1768+
&si.StartupInfo, &pi);
1769+
if (ret && buf.len) {
1770+
errno = err_win_to_posix(GetLastError());
1771+
warning("failed to restrict file handles (%ld)\n\n%s",
1772+
err, buf.buf);
1773+
}
1774+
strbuf_release(&buf);
1775+
} else if (!ret)
1776+
errno = err_win_to_posix(GetLastError());
1777+
1778+
if (si.lpAttributeList)
1779+
DeleteProcThreadAttributeList(si.lpAttributeList);
1780+
if (attr_list)
1781+
HeapFree(GetProcessHeap(), 0, attr_list);
16831782

16841783
free(wenvblk);
16851784
free(wargs);
16861785

1687-
if (!ret) {
1688-
errno = ENOENT;
1786+
if (!ret)
16891787
return -1;
1690-
}
1788+
16911789
CloseHandle(pi.hThread);
16921790

16931791
/*

‎compat/winansi.c‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,10 +660,20 @@ void winansi_init(void)
660660
*/
661661
HANDLE winansi_get_osfhandle(int fd)
662662
{
663+
HANDLE ret;
664+
663665
if (fd == 1 && (fd_is_interactive[1] & FD_SWAPPED))
664666
return hconsole1;
665667
if (fd == 2 && (fd_is_interactive[2] & FD_SWAPPED))
666668
return hconsole2;
667669

668-
return (HANDLE)_get_osfhandle(fd);
670+
ret = (HANDLE)_get_osfhandle(fd);
671+
672+
/*
673+
* There are obviously circumstances under which _get_osfhandle()
674+
* returns (HANDLE)-2. This is not documented anywhere, but that is so
675+
* clearly an invalid handle value that we can just work around this
676+
* and return the correct value for invalid handles.
677+
*/
678+
return ret == (HANDLE)-2 ? INVALID_HANDLE_VALUE : ret;
669679
}

‎t/helper/test-run-command.c‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,58 @@ static int task_finished(int result,
5050
return 1;
5151
}
5252

53+
static int inherit_handle(const char *argv0)
54+
{
55+
struct child_process cp = CHILD_PROCESS_INIT;
56+
char path[PATH_MAX];
57+
int tmp;
58+
59+
/* First, open an inheritable handle */
60+
xsnprintf(path, sizeof(path), "out-XXXXXX");
61+
tmp = xmkstemp(path);
62+
63+
argv_array_pushl(&cp.args,
64+
"test-tool", argv0, "inherited-handle-child", NULL);
65+
cp.in = -1;
66+
cp.no_stdout = cp.no_stderr = 1;
67+
if (start_command(&cp) < 0)
68+
die("Could not start child process");
69+
70+
/* Then close it, and try to delete it. */
71+
close(tmp);
72+
if (unlink(path))
73+
die("Could not delete '%s'", path);
74+
75+
if (close(cp.in) < 0 || finish_command(&cp) < 0)
76+
die("Child did not finish");
77+
78+
return 0;
79+
}
80+
81+
static int inherit_handle_child(void)
82+
{
83+
struct strbuf buf = STRBUF_INIT;
84+
85+
if (strbuf_read(&buf, 0, 0) < 0)
86+
die("Could not read stdin");
87+
printf("Received %s\n", buf.buf);
88+
strbuf_release(&buf);
89+
90+
return 0;
91+
}
92+
5393
int cmd__run_command(int argc, const char **argv)
5494
{
5595
struct child_process proc = CHILD_PROCESS_INIT;
5696
int jobs;
5797

98+
if (argc < 2)
99+
return 1;
100+
if (!strcmp(argv[1], "inherited-handle"))
101+
exit(inherit_handle(argv[0]));
102+
if (!strcmp(argv[1], "inherited-handle-child"))
103+
exit(inherit_handle_child());
104+
58105
if (argc < 3)
59106
return 1;
60107
while (!strcmp(argv[1], "env")) {

‎t/t0061-run-command.sh‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ cat >hello-script <<-EOF
1212
cat hello-script
1313
EOF
1414

15+
test_expect_success MINGW 'subprocess inherits only std handles' '
16+
test-tool run-command inherited-handle
17+
'
18+
1519
test_expect_success 'start_command reports ENOENT' '
1620
test-tool run-command start-command-ENOENT ./does-not-exist
1721
'

0 commit comments

Comments
 (0)