public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
From: Corinna Vinschen <corinna@sourceware.org>
To: cygwin-cvs@sourceware.org
Subject: [newlib-cygwin] Cygwin: fork: fix child process permissions, take 2
Date: Wed, 30 Jan 2019 11:19:00 -0000	[thread overview]
Message-ID: <20190130111915.5138.qmail@sourceware.org> (raw)

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

commit ef8ce3077f55d5a105b39f605b877da50ab80aa7
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Wed Jan 30 12:18:03 2019 +0100

    Cygwin: fork: fix child process permissions, take 2
    
    VirtualQueryEx, called by fixup_mmaps_after_fork, requires
    PROCESS_QUERY_INFORMATION permissions per MSDN.  However, testing
    shows that PROCESS_QUERY_LIMITED_INFORMATION is sufficient when
    running the same code on Windows 8.1 or Windows 10.  Fix the code
    to give the forked child always PROCESS_QUERY_INFORMATION perms
    on Windows Vista/7 and respective server releases.
    
    Revert now unneeded patch to check_token_membership as well.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/security.h |  1 -
 winsup/cygwin/sigproc.cc | 10 ++++------
 winsup/cygwin/uinfo.cc   | 14 ++++----------
 3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h
index ec68171..70912b4 100644
--- a/winsup/cygwin/security.h
+++ b/winsup/cygwin/security.h
@@ -17,7 +17,6 @@ details. */
 
 /* UID/GID */
 void uinfo_init ();
-bool check_token_membership (HANDLE, PSID);
 bool check_token_membership (PSID);
 
 #define ILLEGAL_UID ((uid_t)-1)
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 080fe58..a830bff 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -819,12 +819,10 @@ child_info::child_info (unsigned in_cb, child_info_types chtype,
   if (type == _CH_FORK)
     {
       perms |= PROCESS_DUP_HANDLE;
-      /* For some reason fork on Windows 7 requires PROCESS_QUERY_INFORMATION
-	 rather than just PROCESS_QUERY_LIMITED_INFORMATION when started as a
-	 service. */
-      if (wincap.needs_query_information ()
-	  && (cygheap->user.saved_sid () == well_known_system_sid
-	      || check_token_membership (hProcToken, well_known_service_sid)))
+      /* VirtualQueryEx is documented to require PROCESS_QUERY_INFORMATION.
+	 That's true for Windows 7, but PROCESS_QUERY_LIMITED_INFORMATION
+	 appears to be sufficient on Windows 8 and later. */
+      if (wincap.needs_query_information ())
 	perms |= PROCESS_QUERY_INFORMATION;
     }
 
diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 00a2b5a..8dcf731 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -118,13 +118,16 @@ cygheap_user::init ()
    This needs careful checking should we use check_token_membership in other
    circumstances. */
 bool
-check_token_membership (HANDLE tok, PSID sid)
+check_token_membership (PSID sid)
 {
   NTSTATUS status;
   ULONG size;
   tmp_pathbuf tp;
   PTOKEN_GROUPS groups = (PTOKEN_GROUPS) tp.w_get ();
 
+  /* If impersonated, use impersonation token. */
+  HANDLE tok = cygheap->user.issetuid () ? cygheap->user.primary_token ()
+					 : hProcToken;
   status = NtQueryInformationToken (tok, TokenGroups, groups, 2 * NT_MAX_PATH,
 				    &size);
   if (!NT_SUCCESS (status))
@@ -139,15 +142,6 @@ check_token_membership (HANDLE tok, PSID sid)
   return false;
 }
 
-bool
-check_token_membership (PSID sid)
-{
-  /* If impersonated, use impersonation token. */
-  HANDLE tok = cygheap->user.issetuid () ? cygheap->user.primary_token ()
-					 : hProcToken;
-  return check_token_membership (tok, sid);
-}
-
 static void
 internal_getlogin (cygheap_user &user)
 {


                 reply	other threads:[~2019-01-30 11:19 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=20190130111915.5138.qmail@sourceware.org \
    --to=corinna@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).