From patchwork Mon Dec 2 11:33:28 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: 11269053 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 D2D7917EF for ; Mon, 2 Dec 2019 11:33:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B1704215E5 for ; Mon, 2 Dec 2019 11:33:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="gEX3zPoz" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727419AbfLBLdf (ORCPT ); Mon, 2 Dec 2019 06:33:35 -0500 Received: from mail-wm1-f68.google.com ([209.85.128.68]:35305 "EHLO mail-wm1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727406AbfLBLde (ORCPT ); Mon, 2 Dec 2019 06:33:34 -0500 Received: by mail-wm1-f68.google.com with SMTP id u8so7301587wmu.0 for ; Mon, 02 Dec 2019 03:33:32 -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=VHcKo6z3SUU9qF3WHhAITjhsf0gwlwLshM5Ogo6j27w=; b=gEX3zPozcgAZUW0MWDwfOhCt5cV8nO6ApKJUJk/XXjvjJHJfDzOQSWEvHnE4zSNxvX A5rpvdPgdImmoj3sqxj+nGenblnmJPCYeA9SNybIpYPwW+h+7c6LoRh6/J9PzWZc/4gO H1z9YnX4BRLogo0/us+Pf0vOcqJ5u6pWVg2AWKMuAnx8n5YgqKZ0d5F6LdhgZ7Ee2DRR 5T/ARZm1ciLhufKLLvVuL6gvjAVrgo0tvt3fo7+DgCll9Y/7iTRnc2XfaJP+D58ovtQk U5D2HOFEsXOdMfBF35xtn4nEqcnB8NvRyIWD7nWGgCl57UzYm3XVslsZv0D8ODxTATwc 8N8A== 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=VHcKo6z3SUU9qF3WHhAITjhsf0gwlwLshM5Ogo6j27w=; b=a6sW4zzgbcLn9ywVplL6ejaz0z38G0akBVGQwdPWraTtY9JuHTF466WIm0UhhglpCj YtbZrZiupMjpcXAhw5x2l+DjaGESfLwdkrtt5An7y1VIk6kwv11kpy9+xxi0dfX+YjJm 8BNIydDNl6IQXsrSdaRKo0HohNgknTuqna/6MXwFj8g7EuvDAP174BrZIxC8IhM+rWfl QimA/DCT8RY9cdtpeBIX4DqA44EIuI0kPisRH52JiCAUVwYv9WRGr9/DhIzRKOeQGHIj 5f2Z87soxb6BKX53f3gmX0LmhncRrxfN8YlTi+rdEvMHCO5lG/hT3l/VbWKagKpsvTue e5wg== X-Gm-Message-State: APjAAAVpXLjDs1d5xoGKonGBMXfCyr7rVXzGGksImA8VGfqvXwrWeCbI ivH7xm10nI0K5lRn8AVJgSFtAx6C X-Google-Smtp-Source: APXvYqxBEGFvGUbKQMjgZg/si0Hbeu+PZUZFAfaPIatRuvBrmgPQE88lFKn/22AJLFJNDoQvSINd5w== X-Received: by 2002:a7b:c632:: with SMTP id p18mr18824141wmk.175.1575286411226; Mon, 02 Dec 2019 03:33:31 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id b17sm14891288wrx.15.2019.12.02.03.33.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 02 Dec 2019 03:33:30 -0800 (PST) Message-Id: <280b6d08a4cdae7af168371670f41585aa4778d4.1575286409.git.gitgitgadget@gmail.com> In-Reply-To: References: From: "Johannes Schindelin via GitGitGadget" Date: Mon, 02 Dec 2019 11:33:28 +0000 Subject: [PATCH v3 1/2] mingw: do set `errno` correctly when trying to restrict handle inheritance Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Sixt , 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 In 9a780a384de (mingw: spawned processes need to inherit only standard handles, 2019-11-22), we taught the Windows-specific part to restrict which file handles are passed on to the spawned processes. Since this logic seemed to be a bit fragile across Windows versions (we _still_ support Windows Vista in Git for Windows, for example), a fall-back was added to try spawning the process again, this time without restricting which file handles are to be inherited by the spawned process. In the common case (i.e. when the process could not be spawned for reasons _other_ than the file handle inheritance), the fall-back attempt would still fail, of course. Crucially, one thing we missed in that code path was to set `errno` appropriately. This should have been caught by t0061.2 which expected `errno` to be `ENOENT` after trying to start a process for a non-existing executable, but `errno` was set to `ENOENT` prior to the `CreateProcessW()` call: while looking for the config settings for trace2, Git tries to access `xdg_config` and `user_config` via `access_or_die()`, and as neither of those config files exists when running the test case (because in Git's test suite, `HOME` points to the test directory), the `errno` has the expected value, but for the wrong reasons. Let's fix that by making sure that `errno` is set correctly. It even appears that `errno` was set in the _wrong_ case previously: `CreateProcessW()` returns non-zero upon success, but `errno` was set only in the non-zero case. It would be nice if we could somehow fix t0061 to make sure that this does not regress again. One approach that seemed like it should work, but did not, was to set `errno` to 0 in the test helper that is used by t0061.2. However, when `mingw_spawnvpe()` wants to see whether the file in question is a script, it calls `parse_interpreter()`, which in turn tries to `open()` the file. Obviously, this call fails, and sets `errno` to `ENOENT`, deep inside the call chain started from that test helper. Instead, we force re-set `errno` at the beginning of the function `mingw_spawnve_fd()`, which _should_ be safe given that callers of that function will want to look at `errno` if -1 was returned. And if that `errno` is 0 ("No error"), regression tests like t0061.2 will kick in. Reported-by: Johannes Sixt Signed-off-by: Johannes Schindelin --- compat/mingw.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compat/mingw.c b/compat/mingw.c index 2b6eca2f56..432adc1aed 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1423,6 +1423,9 @@ 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; + /* Make sure to override previous errors, if any */ + errno = 0; + if (restrict_handle_inheritance < 0) restrict_handle_inheritance = core_restrict_inherited_handles; /* @@ -1580,8 +1583,9 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen ret = CreateProcessW(*wcmd ? wcmd : NULL, wargs, NULL, NULL, TRUE, flags, wenvblk, dir ? wdir : NULL, &si.StartupInfo, &pi); - if (ret && buf.len) { + 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); } From patchwork Mon Dec 2 11:33:29 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: 11269051 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 A7A3A159A for ; Mon, 2 Dec 2019 11:33:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 86195215E5 for ; Mon, 2 Dec 2019 11:33:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="RTjLOR2k" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727417AbfLBLde (ORCPT ); Mon, 2 Dec 2019 06:33:34 -0500 Received: from mail-wm1-f67.google.com ([209.85.128.67]:38374 "EHLO mail-wm1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727401AbfLBLde (ORCPT ); Mon, 2 Dec 2019 06:33:34 -0500 Received: by mail-wm1-f67.google.com with SMTP id p17so9991513wmi.3 for ; Mon, 02 Dec 2019 03:33:32 -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=ZcVrR64j5Wkx6FSjayBramr2+xYCxktni1aiRnccl2Y=; b=RTjLOR2kFRNgSa62CC2S06fCWUzNZSlBAvI1LrTUhlfMdDhl07xRgPAs4udvP/pkcf hFpcod6KvSMoYZDC5oLgzYoJCAw2CGhUkBj+zF4TGhBXf/dObuXkjipn+qUph3FYao3R 8TbQkFghTpM9R+/yNmTr/0PNHix4ST1QOG7d3L90D9IVdNJelPY4wK+Rl154wmzo35t3 B5/sScuCC0unzV76gDxa15STNFee4mMcw04YpKqWivPeLNCTVqkbGQlWaGt/AFncx6Nq ystFWyofpfPMbvs6S1a8Gev0GZ1iaFwuu3srqbJn1RZuxlk5imj/A6qfeeYq+ZoL+1lX 9hMg== 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=ZcVrR64j5Wkx6FSjayBramr2+xYCxktni1aiRnccl2Y=; b=g6cFj8e2jPBqb4pQDAzotN0m8x0zaQkBQuSSdtb7Zpzt9liR1kVBx87E3Os9vA474E t8U2JKMX8cB9RnNx9oSH6H/XlR7lyEdMgRcC5cmuQDBwL93YYOlkAPqdDGgPax08A3n4 GETycghn+rB16oDMPimPXrP5TC4rporrMUgm7Gx3M+gdd8kVpNXIW/SqDrpcXxhhItmQ mfe3r2jIEP3DiGAEI7DYt8UQk7JAWYiJxWrFruIJwVfNBDONtdWTd4rW6McTxp+ZMem4 dMpqWD6pryfFMgORrRN98h413YkI0KFLnr+6C/bIPKgw3oOM5onhDXq6lDSsOKX1p1so 5Qpw== X-Gm-Message-State: APjAAAUR8y32VWYjSGCESInMv7kDCExPCcs1/oWsciOCYyww+/mPdKye ikzozFb1erqtjHzFfhHdX6xifjmp X-Google-Smtp-Source: APXvYqzj0qM2AxryHrOO4nT+Lv1Da9A7a1uesUkXAS+bEYblgdeoFlfh+bBforJllMf5ic8rrDn2ug== X-Received: by 2002:a05:600c:2144:: with SMTP id v4mr27735287wml.141.1575286412150; Mon, 02 Dec 2019 03:33:32 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id j184sm6226500wmb.44.2019.12.02.03.33.31 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 02 Dec 2019 03:33:31 -0800 (PST) Message-Id: In-Reply-To: References: From: "Johannes Schindelin via GitGitGadget" Date: Mon, 02 Dec 2019 11:33:29 +0000 Subject: [PATCH v3 2/2] mingw: translate ERROR_SUCCESS to errno = 0 Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Sixt , 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 Johannes Sixt pointed out that the `err_win_to_posix()` function mishandles `ERROR_SUCCESS`: it maps it to `ENOSYS`. The only purpose of this function is to map Win32 API errors to `errno` ones, and there is actually no equivalent to `ERROR_SUCCESS`: the idea of `errno` is that it will only be set in case of an error, and left alone in case of success. Therefore, as pointed out by Junio Hamano, it is a bug to call this function when there was not even any error to map. Signed-off-by: Johannes Schindelin --- compat/mingw.c | 1 + 1 file changed, 1 insertion(+) diff --git a/compat/mingw.c b/compat/mingw.c index 432adc1aed..827065d96d 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -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;