public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
From: Ken Brown <kbrown@sourceware.org>
To: cygwin-cvs@sourceware.org
Subject: [newlib-cygwin] Cygwin: fhandler_pipe::get_query_hdl_per_process: avoid a crash
Date: Wed, 29 Dec 2021 20:26:21 +0000 (GMT)	[thread overview]
Message-ID: <20211229202621.06520385843F@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=0ce992c1e40f843d5f264e87908a6decfd6681d1

commit 0ce992c1e40f843d5f264e87908a6decfd6681d1
Author: Ken Brown <kbrown@cornell.edu>
Date:   Sun Dec 26 16:42:26 2021 -0500

    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

Diff:
---
 winsup/cygwin/fhandler_pipe.cc | 13 +++++++++++--
 winsup/cygwin/release/3.3.4    |  3 +++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index ba6b70f55..aef0bf6be 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -6,8 +6,6 @@ This software is a copyrighted work licensed under the terms of the
 Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
 details. */
 
-/* FIXME: Should this really be fhandler_pipe.cc? */
-
 #include "winsup.h"
 #include <stdlib.h>
 #include <sys/socket.h>
@@ -20,6 +18,7 @@ details. */
 #include "pinfo.h"
 #include "shared_info.h"
 #include "tls_pbuf.h"
+#include <assert.h>
 
 /* This is only to be used for writing.  When reading,
 STATUS_PIPE_EMPTY simply means there's no data to be read. */
@@ -1228,6 +1227,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/05f5e9fa477dcaa1709d9518170d18e1b3b8330d/phlib/native.c#L5754.
+	     We need to ensure that NumberOfHandles is zero in this
+	     case to avoid a crash in the for loop below. */
+	  phi->NumberOfHandles = 0;
 	  status = NtQueryInformationProcess (proc, ProcessHandleInformation,
 					      phi, nbytes, &len);
 	  if (NT_SUCCESS (status))
@@ -1239,6 +1244,10 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
       if (!NT_SUCCESS (status))
 	goto close_proc;
 
+      /* Sanity check in case Microsoft changes
+	 NtQueryInformationProcess and the initialization of
+	 NumberOfHandles above is no longer sufficient. */
+      assert (phi->NumberOfHandles <= n_handle);
       for (ULONG j = 0; j < phi->NumberOfHandles; j++)
 	{
 	  /* Check for the peculiarity of cygwin read pipe */
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


                 reply	other threads:[~2021-12-29 20:26 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211229202621.06520385843F@sourceware.org \
    --to=kbrown@sourceware.org \
    --cc=cygwin-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).