On 12/26/2021 4:35 PM, Jeremy Drake wrote:
On Sun, 26 Dec 2021, Ken Brown wrote:
On 12/26/2021 11:04 AM, Ken Brown wrote:
On 12/26/2021 10:09 AM, Ken Brown wrote:
1. For some processes, NtQueryInformationProcess(ProcessHandleInformation)
can return STATUS_SUCCESS with invalid handle information. See the
comment starting at line 5754, where it is shown how to detect this.
I kind of thought something like this (that NumberOfHandles was
uninitialized memory).
If I'm right, the following patch should fix the problem:
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index ba6b70f55..4cef3e4ca 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -1228,6 +1228,7 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
HeapAlloc (GetProcessHeap (), 0, nbytes);
if (!phi)
goto close_proc;
+ phi->NumberOfHandles = 0;
status = NtQueryInformationProcess (proc,
ProcessHandleInformation,
phi, nbytes, &len);
if (NT_SUCCESS (status))
Actually, this first hunk should suffice.
Jeremy, could you try this?
Ken
I've built (leaving the assert in place too), and I've got 3 loops going
on server 2022 and 1 going on ARM64. So far so good. I don't know how
long before calling it good though.
Great, thanks for testing. I'm attaching the complete patch (with
documentation). I'll push it once you're convinced that it fixes the problem,
assuming Takashi agrees. (I think Corinna is unavailable.)
Ken
From 4858e73321a0618a8b1e1060416ef7d546cda895 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Sun, 26 Dec 2021 16:42:26 -0500
Subject: [PATCH] Cygwin: fhandler_pipe::get_query_hdl_per_process: avoid a
crash
NtQueryInformationProcess(ProcessHandleInformation) can return
STATUS_SUCCESS with invalid handle data for certain processes
("minimal" processes on Windows 10). This can cause a crash when
there's an attempt to access that data. Fix that by setting
NumberOfHandles to zero before calling NtQueryInformationProcess.
Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q4/011611.html
---
winsup/cygwin/fhandler_pipe.cc | 6 ++++++
winsup/cygwin/release/3.3.4 | 3 +++
2 files changed, 9 insertions(+)
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index 25a092262..2674d154c 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -1256,6 +1256,12 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
HeapAlloc (GetProcessHeap (), 0, nbytes);
if (!phi)
goto close_proc;
+ /* NtQueryInformationProcess can return STATUS_SUCCESS with
+ invalid handle data for certain processes. See
+
https://github.com/processhacker/processhacker/blob/master/phlib/native.c#L5754.
+ We need to ensure that NumberOfHandles is zero in this
+ case to avoid a crash in the loop below. */
+ phi->NumberOfHandles = 0;
status = NtQueryInformationProcess (proc, ProcessHandleInformation,
phi, nbytes, &len);
if (NT_SUCCESS (status))
diff --git a/winsup/cygwin/release/3.3.4 b/winsup/cygwin/release/3.3.4
index a15684fdb..048426942 100644
--- a/winsup/cygwin/release/3.3.4
+++ b/winsup/cygwin/release/3.3.4
@@ -14,3 +14,6 @@ Bug Fixes
rather than io_handle while neither read() nor select() is called
after the cygwin app is started from non-cygwin app.
Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q4/011587.html
+
+- Avoid a crash when NtQueryInformationProcess returns invalid handle data.
+ Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q4/011611.html
--
2.34.1
