From patchwork Fri Nov 22 14:41:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Arver via GitGitGadget X-Patchwork-Id: 11258059 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 59AC21593 for ; Fri, 22 Nov 2019 14:41:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 39D0E2071B for ; Fri, 22 Nov 2019 14:41:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="n9R8kM13" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726760AbfKVOlK (ORCPT ); Fri, 22 Nov 2019 09:41:10 -0500 Received: from mail-wr1-f67.google.com ([209.85.221.67]:38124 "EHLO mail-wr1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726613AbfKVOlK (ORCPT ); Fri, 22 Nov 2019 09:41:10 -0500 Received: by mail-wr1-f67.google.com with SMTP id i12so8906603wro.5 for ; Fri, 22 Nov 2019 06:41:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=IxSqxarUv44dtRuXTg+T1t2Qk7SPpmGt/f4uvGizr6g=; b=n9R8kM13dvt+nuLcn3L1BIhpSmbdO8FRwBtggGnhslUMvfsOXAnEdNasnUb5xQv5Uj Enx9dBNAxI4MFg4X8Tlsfl98QS2uwiF+D8WSSttgM+RV8zPsaFbsdO+83OQOeU9KX0aW RJGIw2DuGOyZw76cBPwy/4fOp0FuBFQe/qmRYEJ1EaM3QVte94YNFTQ8CPGYWI/rIx3N U1xRqrM/vjX+A8573sY3rNwSi3Y+MAP+KUXtV7DRAFBemsP/AA2tEskaQzTHHnRHPQ3T MbMGdnKR3gFuWFzAvz2rVJHxIz71IN5wQZb36fSnWDfPjjS7mpmI5rKPA2TVa1PUmeeq KbOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=IxSqxarUv44dtRuXTg+T1t2Qk7SPpmGt/f4uvGizr6g=; b=NcWfvkGvPz3bYsXks3mLacNN61BP2L+wWuxEP34UCpVz0OQj018HZJ502ZeA/eB0Om ZjwzD9S7zBSr0nN+sLp5Fy/h10ckQsKq9e/zPQsgxHs11RifnAt4qNGuWR8MpibVNZSv U4H//Z5jRE0OQQrSMcD8F/0H+MhEi+pP8hDfBXl2IrdX+Go5EAUbXqfrZFm6YU+WaLdF AvZQ32rz0uG0lvPs8ZHpAoGRdF7uZTMlt6TgbfsKSAU6ONmtnF1+2KgO1Sh44NrfIO2/ 6QesfjP1yjpiclTvWQoPi3d/9DoU1ynL7HjbuZzqDJ/hDV29XpXn/MAWTffpbg2NU/tp Yp8g== X-Gm-Message-State: APjAAAW7QrUzKuj1C5Yl8YhAkq3peKpF4K0bXh45HUd4iEyUZLjjOGq0 2gbgMeynhDgjrkwh4z+xcRHbcuH4 X-Google-Smtp-Source: APXvYqw0pjVJloqBFe4LEwUX6qAdp1YhJxnKs3j4om3ZH6SNbHIEM8UVEBP3FFJEMeEtBlbnOJcRgw== X-Received: by 2002:a5d:678f:: with SMTP id v15mr17466991wru.242.1574433667677; Fri, 22 Nov 2019 06:41:07 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id y78sm3756680wmd.32.2019.11.22.06.41.07 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 22 Nov 2019 06:41:07 -0800 (PST) Message-Id: <96608fd2e82b7337dea4ea9fb7a2f3f1f7e6c854.1574433665.git.gitgitgadget@gmail.com> In-Reply-To: References: From: "Johannes Schindelin via GitGitGadget" Date: Fri, 22 Nov 2019 14:41:02 +0000 Subject: [PATCH 1/4] mingw: demonstrate that all file handles are inherited by child processes Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Johannes Schindelin Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Johannes Schindelin When spawning child processes, we really should be careful which file handles we let them inherit. This is doubly important on Windows, where we cannot rename, delete, or modify files if there is still a file handle open. Signed-off-by: Johannes Schindelin --- t/helper/test-run-command.c | 44 +++++++++++++++++++++++++++++++++++++ t/t0061-run-command.sh | 4 ++++ 2 files changed, 48 insertions(+) diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index ead6dc611a..40ec4dbb6e 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -200,6 +200,46 @@ static int testsuite(int argc, const char **argv) return !!ret; } +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; @@ -207,6 +247,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 < 3) return 1; diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 17c9c0f3bb..473a3405ef 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -12,6 +12,10 @@ cat >hello-script <<-EOF cat hello-script EOF +test_expect_failure 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 From patchwork Fri Nov 22 14:41:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Arver via GitGitGadget X-Patchwork-Id: 11258057 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id EB79A138C for ; Fri, 22 Nov 2019 14:41:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CAC5720715 for ; Fri, 22 Nov 2019 14:41:11 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="dREM6uWA" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726836AbfKVOlL (ORCPT ); Fri, 22 Nov 2019 09:41:11 -0500 Received: from mail-wm1-f66.google.com ([209.85.128.66]:51279 "EHLO mail-wm1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726634AbfKVOlK (ORCPT ); Fri, 22 Nov 2019 09:41:10 -0500 Received: by mail-wm1-f66.google.com with SMTP id g206so7445138wme.1 for ; Fri, 22 Nov 2019 06:41:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=a5/4SnLiOBteb8N0+2klqseFF0BuFqll9XXKoppPzIg=; b=dREM6uWAcDb9fqV6gjBeeKzrn4MYuZE0lwgZ3mmdmR+OIM3YuWpVV6HGg58QDnQAaR HJ9ykw3g3/+5kmYVPB85Y+XG+f/RS6+w6JtATouSbkGmScNCDDhF0Wb/TSmUJbvM3DPV rhGTuy14/IzlLpFDCkqdIRhzJ7zdSzzEwL15MjPnzhwZB5XxQ3jUuoG8MhfBmz6MckXR AHaPCOigP4+DtS/JFJAfGBdTo5gHfxY4nQTgHOit3ztrUjJE6oXVUlEsYfTBbHZ4FKBc f7fkkh515vlxCTfR5/4lS7FJMff6Q0T7/akw9lfMV86sn4Vbvp85EAw6l+2PozJpNHiI BebQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=a5/4SnLiOBteb8N0+2klqseFF0BuFqll9XXKoppPzIg=; b=X/EUSx+PJw+d/3+0nqMUaIfklhaISPw0gd74/LApNlaoxmWO9ufknAfNurbCEZ42X9 xACcodLVRWi5jBfjBltHss3KVHIf0LSPZxjy62Wx2rpuxX+2qbzE8wlp9toj9C0GVOyh Ajm0u1ACgWkPRNlXHzkFCEg8kE4vbtvdFz/fYlczibnJBSR6M5+0tt34gdpOKp1nKgI3 XNNQBQhhX6yh1LnYVSgTBY4/QgHoS1f0eM/aGJ+gW9krR1ai83fP9pk+r3GlbzsSJcj2 Meer0WIPRrgpvGFbxuJxzuKB5NPjF5a7AmufEPpyGNYS4Xej6eK2UJI3zKQrm5BHhMRU Nw2Q== X-Gm-Message-State: APjAAAVS4STHv7VOPWBDzjaS8bwbkBfe5819ZNRSifCPd8uX7gOxdHiQ RURuoi+0PHQC2Iq9H30mG4Krg905 X-Google-Smtp-Source: APXvYqyCYfLopIFM1wiSfONEn0NROB6Zq9f4Z/ZS1MTFV7JhcYXFrX9tUqydPJnsoeioInXSfGIWNw== X-Received: by 2002:a05:600c:230d:: with SMTP id 13mr10445718wmo.12.1574433668525; Fri, 22 Nov 2019 06:41:08 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id d67sm3648694wmd.13.2019.11.22.06.41.07 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 22 Nov 2019 06:41:08 -0800 (PST) Message-Id: In-Reply-To: References: From: "Johannes Schindelin via GitGitGadget" Date: Fri, 22 Nov 2019 14:41:03 +0000 Subject: [PATCH 2/4] mingw: work around incorrect standard handles Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Johannes Schindelin Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Johannes Schindelin For some reason, when being called via TortoiseGit the standard handles, or at least what is returned by _get_osfhandle(0) for standard input, can take on the value (HANDLE)-2 (which is not a legal value, according to the documentation). Even if this value is not documented anywhere, CreateProcess() seems to work fine without complaints if hStdInput set to this value. In contrast, the upcoming code to restrict which file handles get inherited by spawned processes would result in `ERROR_INVALID_PARAMETER` when including such handle values in the list. To help this, special-case the value (HANDLE)-2 returned by _get_osfhandle() and replace it with INVALID_HANDLE_VALUE, which will hopefully let the handle inheritance restriction work even when called from TortoiseGit. This fixes https://github.com/git-for-windows/git/issues/1481 Signed-off-by: Johannes Schindelin --- compat/winansi.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/compat/winansi.c b/compat/winansi.c index 54fd701cbf..c27b20a79d 100644 --- a/compat/winansi.c +++ b/compat/winansi.c @@ -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; } From patchwork Fri Nov 22 14:41:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Arver via GitGitGadget X-Patchwork-Id: 11258063 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id BB3D66C1 for ; Fri, 22 Nov 2019 14:41:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 924112071F for ; Fri, 22 Nov 2019 14:41:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Uj1qfPPT" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726895AbfKVOlO (ORCPT ); Fri, 22 Nov 2019 09:41:14 -0500 Received: from mail-wr1-f68.google.com ([209.85.221.68]:44379 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726546AbfKVOlM (ORCPT ); Fri, 22 Nov 2019 09:41:12 -0500 Received: by mail-wr1-f68.google.com with SMTP id i12so8852785wrn.11 for ; Fri, 22 Nov 2019 06:41:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=Fd0RJytb8yE0ezMrwNyNzPh/LCYv/WmTwTPXUFSgMpE=; b=Uj1qfPPTHsti0z0i0EErJWX4KGydsVGZQKq4rvm25shQB7a+RGnp1Xxwp7vOTZKjEd 8VLmCbwlBeBS86aahSXhsMs/zlpTbAJm9Nmje7ziCFfeWIozyTO/OThSSO4qb60+nvDk Xx+GsUwm8nzmkj9J1spPa4JoCroIlAJp11Krhey7iH8buVENSWQMVZvwwk7MaEudE2SU oz7H+xTrj5JxYXrwyuPtHMC7jbuGfseABbpVdx161JEDE0OxbK205gPTewC47mC9lCtx 7iS/sdCrqd8waEOX/s5kke+orbJEalXVZ+qKxPyemNzUM+hdK8Nd5oAwBrQPhAsoO7JX fi7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=Fd0RJytb8yE0ezMrwNyNzPh/LCYv/WmTwTPXUFSgMpE=; b=lhncDZWo/+APMvXg7+LCNh+eLulAT+8pYWL7bSnWHTQZ/uuFHrcK552HAdRlFqcWvg ifAsvYZ+1PqutXTnYjb5ZUB06dK2TFFyb/g6ZVnGTOcFbgtsZWtWLWOCxClol5LOGc7Z JQr9Y/QNQIDbhMMmFtqSVdx6GFMMy0vIdcf5g7FDdfIl1xiMhhT8POTsxxYSmG81NITG XmIUx3vKBqJ6bsn4YR6y3FC0imfMArWff6A1mXf0WH5+jBR1tbcRKWXE4T5kjubSZQ2w kAHEL2VokG+8qGpoYcx/ndiXPWnAwJLAMVP4MHNyvJT9PuOXphpPkMjdO5NoTr/wyRD1 gzqg== X-Gm-Message-State: APjAAAVfiHGKQAly61hnFaPjuWdKMB+747+wlLesKKfIfgO9Yvq8t5NP nWBcVscOw0w540BdfLRAEfJwjiIG X-Google-Smtp-Source: APXvYqxXJFoEc6FerHTjcW+q6D+g66PVt8nELSBhHmbvEcKmcHQv5o52HlGTmaO8OzzQuNU/4BLiyw== X-Received: by 2002:a5d:6585:: with SMTP id q5mr17565687wru.158.1574433669325; Fri, 22 Nov 2019 06:41:09 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id y8sm8201863wru.59.2019.11.22.06.41.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 22 Nov 2019 06:41:08 -0800 (PST) Message-Id: In-Reply-To: References: From: "Johannes Schindelin via GitGitGadget" Date: Fri, 22 Nov 2019 14:41:04 +0000 Subject: [PATCH 3/4] mingw: spawned processes need to inherit only standard handles Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Johannes Schindelin Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Johannes Schindelin By default, CreateProcess() does not inherit any open file handles, unless the bInheritHandles parameter is set to TRUE. Which we do need to set because we need to pass in stdin/stdout/stderr to talk to the child processes. Sadly, this means that all file handles (unless marked via O_NOINHERIT) are inherited. This lead to problems in VFS for Git, where a long-running read-object hook is used to hydrate missing objects, and depending on the circumstances, might only be called *after* Git opened a file handle. Ideally, we would not open files without O_NOINHERIT unless *really* necessary (i.e. when we want to pass the opened file handle as standard handle into a child process), but apparently it is all-too-easy to introduce incorrect open() calls: this happened, and prevented updating a file after the read-object hook was started because the hook still held a handle on said file. Happily, there is a solution: as described in the "Old New Thing" https://blogs.msdn.microsoft.com/oldnewthing/20111216-00/?p=8873 there is a way, starting with Windows Vista, that lets us define precisely which handles should be inherited by the child process. And since we bumped the minimum Windows version for use with Git for Windows to Vista with v2.10.1 (i.e. a *long* time ago), we can use this method. So let's do exactly that. We need to make sure that the list of handles to inherit does not contain duplicates; Otherwise CreateProcessW() would fail with ERROR_INVALID_ARGUMENT. While at it, stop setting errno to ENOENT unless it really is the correct value. Also, fall back to not limiting handle inheritance under certain error conditions (e.g. on Windows 7, which is a lot stricter in what handles you can specify to limit to). Signed-off-by: Johannes Schindelin --- compat/mingw.c | 120 +++++++++++++++++++++++++++++++++++++---- t/t0061-run-command.sh | 2 +- 2 files changed, 110 insertions(+), 12 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index fe609239dd..cac18cc3da 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1398,8 +1398,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; @@ -1435,11 +1440,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'; @@ -1472,16 +1489,97 @@ 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 && buf.len) { + errno = err_win_to_posix(GetLastError()); + 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); /* diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 473a3405ef..7d599675e3 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -12,7 +12,7 @@ cat >hello-script <<-EOF cat hello-script EOF -test_expect_failure MINGW 'subprocess inherits only std handles' ' +test_expect_success MINGW 'subprocess inherits only std handles' ' test-tool run-command inherited-handle ' From patchwork Fri Nov 22 14:41:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Arver via GitGitGadget X-Patchwork-Id: 11258061 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1C1E96C1 for ; Fri, 22 Nov 2019 14:41:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id F04C12071F for ; Fri, 22 Nov 2019 14:41:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="QTfpaQkg" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726852AbfKVOlO (ORCPT ); Fri, 22 Nov 2019 09:41:14 -0500 Received: from mail-wr1-f51.google.com ([209.85.221.51]:44689 "EHLO mail-wr1-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726613AbfKVOlM (ORCPT ); Fri, 22 Nov 2019 09:41:12 -0500 Received: by mail-wr1-f51.google.com with SMTP id i12so8852832wrn.11 for ; Fri, 22 Nov 2019 06:41:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=m/MnVEB2vVWXY7yuhnRkFNN58n/fRZWdgF5EQrDV7bk=; b=QTfpaQkgTzAkI0/A1U6FuabB9PnVyssMQEcLsXGYuu3wcEbWX/OYrtMAs0tEmO3ie6 l9TGwITb1MDVB0OIIvYkwW3nRDMr/CPUGyrWiRjrrWvrU7XBi7sOUh4UGdPy7Bj8Yd/+ i4Pew0WQMlNoiFcrpVY8UbXXpjZYEMN3p1wdB8+JaLQm4ElW9FuemZaXBELXt5Ntdgw1 RrHBRfpqWR4IWXPySG+SGU3j9P6HQAQqy2U0jsXu/mbp3HBo3dnKgs2APQICINiN6V6M XMAsxfPOofIMbs5N9k3nLqeI4hCPgASLbER1cbNPFQLjn1EdM4xjNB8Rp6yFp0ieJyQy 0TsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=m/MnVEB2vVWXY7yuhnRkFNN58n/fRZWdgF5EQrDV7bk=; b=UmJLl/4kxuhAuKXBX1smAhFzMqdPhCRhosufNA49wg1Tl3LlB6CTQK1CYM9VPynxZs D+4Oq7HMQkJsRnJHfE+QqF4EVMSUtuZCPR8gjERVHLTzVioA66nzRSAeHkZfOSrW2J06 LbRNFZNWjxhSc1ut4yl+XKS7zdrCUlkyhcmiRP4cx6Ht361rP8wI3qBenttayRtKC55Q kQFK+6FayJ7xBV/XyHmve7I4Wi4PFeC7CC5MwxCZdT1dFxn5ypA5O2g1CdYK/s49DvKU 5wt4Fn9SQa+m39f/WGvQvwa7CuAMejuZ0rERtCDm8AWMrYP/wd9UeCDY8pC1gVmTVJA1 +iEg== X-Gm-Message-State: APjAAAWed2/sJslcWxkJ0ssVLEmolwE9Twt7tlGsEquCJs1Z2QXvWVlU hFrflBIoQRiAqQotzdAJMu74G2o9 X-Google-Smtp-Source: APXvYqyPwv4c/rd+D3mNTb/eVoVQ+gNsGkW623NNAhQCbnAw1CB6sCZH6GwafWGc9ebEa3JUrWXcIQ== X-Received: by 2002:a5d:678f:: with SMTP id v15mr17467179wru.242.1574433670123; Fri, 22 Nov 2019 06:41:10 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id c1sm6430902wrs.24.2019.11.22.06.41.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 22 Nov 2019 06:41:09 -0800 (PST) Message-Id: <85adb04e3d9a7a79e9b24ea50be71ac55dff6f65.1574433665.git.gitgitgadget@gmail.com> In-Reply-To: References: From: "Johannes Schindelin via GitGitGadget" Date: Fri, 22 Nov 2019 14:41:05 +0000 Subject: [PATCH 4/4] mingw: restrict file handle inheritance only on Windows 7 and later Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Johannes Schindelin Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Johannes Schindelin Turns out that it don't work so well on Vista, see https://github.com/git-for-windows/git/issues/1742 for details. According to https://devblogs.microsoft.com/oldnewthing/?p=8873, it *should* work on Windows Vista and later. But apparently there are issues on Windows Vista when pipes are involved. Given that Windows Vista is past its end of life (official support ended on April 11th, 2017), let's not spend *too* much time on this issue and just disable the file handle inheritance restriction on any Windows version earlier than Windows 7. Signed-off-by: Johannes Schindelin --- Documentation/config/core.txt | 6 ++++++ compat/mingw.c | 22 +++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt index 852d2ba37a..ad4fa4dccd 100644 --- a/Documentation/config/core.txt +++ b/Documentation/config/core.txt @@ -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 diff --git a/compat/mingw.c b/compat/mingw.c index cac18cc3da..2b6eca2f56 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -212,6 +212,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 +232,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; } @@ -1398,7 +1408,7 @@ 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) { - static int restrict_handle_inheritance = 1; + static int restrict_handle_inheritance = -1; STARTUPINFOEXW si; PROCESS_INFORMATION pi; LPPROC_THREAD_ATTRIBUTE_LIST attr_list = NULL; @@ -1413,6 +1423,16 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen const char *(*quote_arg)(const char *arg) = is_msys2_sh(*argv) ? quote_arg_msys2 : quote_arg_msvc; + 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 */