[newlib-cygwin] Cygwin: fix process parent/child relationship after execve

[newlib-cygwin] Cygwin: fix process parent/child relationship after execve

[Corinna Vinschen]

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

commit 57640bee75d18fac5ce6507f9a2c7a712ca750d8
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Sat Nov 2 12:49:15 2019 +0100

    Cygwin: fix process parent/child relationship after execve
    
    Commit 5a0f2c00aa "Cygwin: fork/exec: fix child process permissions"
    removed the PROCESS_DUP_HANDLE handle permission of the parent process
    handle in the child to avoid a security problem.
    
    It turned out that this broke the following scenario: If a process forks
    and then the parent execs, the child loses the ability to register the
    parent's death.  To wit, after the parent died the child process does
    not set its own PPID to 1 anymore.
    
    The current exec mechanism copies required handle values (handles to
    keep contact to the child processes) into the child_info for the
    about-to-be-exec'ed process.  The exec'ed process is supposed to
    duplicate these handles.  This fails, given that we don't allow the
    exec'ed process PROCESS_DUP_HANDLE access to the exec'ing process since
    commit 5a0f2c00aa.
    
    The fix is to avoid the DuplicateHandle calls in the exec'ed process.
    
    This patch sets the affected handles to "inheritable" in the exec'ing
    process at exec time.  The exec'ed process just copies the handle values
    and resets handle inheritance to "non-inheritable".  The exec'ing
    process doesn't have to reset handle inheritance, it exits after setting
    up the exec'ed process anyway.
    
    Testcase: $ ssh-agent /bin/sleep 3
    
    ssh-agent forks and the parent exec's sleep.  After sleep exits, `ps'
    should show ssh-agent to have PPID 1, and eventually ssh-agent exits.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/pinfo.cc      | 41 ++++++++++++++++++-----------------------
 winsup/cygwin/pinfo.h       |  1 +
 winsup/cygwin/release/3.1.0 |  3 +++
 winsup/cygwin/sigproc.cc    |  3 +++
 4 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 35c1ffe..800adbf 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -478,33 +478,28 @@ pinfo::set_acl()
     debug_printf ("NtSetSecurityObject %y", status);
 }
 
+void
+pinfo_minimal::set_inheritance (bool inherit)
+{
+  DWORD i_flag = inherit ? HANDLE_FLAG_INHERIT : 0;
+
+  SetHandleInformation (rd_proc_pipe, HANDLE_FLAG_INHERIT, i_flag);
+  SetHandleInformation (hProcess, HANDLE_FLAG_INHERIT, i_flag);
+  SetHandleInformation (h, HANDLE_FLAG_INHERIT, i_flag);
+}
+
 pinfo::pinfo (HANDLE parent, pinfo_minimal& from, pid_t pid):
   pinfo_minimal (), destroy (false), procinfo (NULL), waiter_ready (false),
   wait_thread (NULL)
 {
-  HANDLE herr;
-  const char *duperr = NULL;
-  if (!DuplicateHandle (parent, herr = from.rd_proc_pipe, GetCurrentProcess (),
-			&rd_proc_pipe, 0, false, DUPLICATE_SAME_ACCESS))
-    duperr = "couldn't duplicate parent rd_proc_pipe handle %p for forked child %d after exec, %E";
-  else if (!DuplicateHandle (parent, herr = from.hProcess, GetCurrentProcess (),
-			     &hProcess, 0, false, DUPLICATE_SAME_ACCESS))
-    duperr = "couldn't duplicate parent process handle %p for forked child %d after exec, %E";
-  else
-    {
-      h = NULL;
-      DuplicateHandle (parent, from.h, GetCurrentProcess (), &h, 0, false,
-		       DUPLICATE_SAME_ACCESS);
-      init (pid, PID_MAP_RW, h);
-      if (*this)
-	return;
-    }
-
-  if (duperr)
-    debug_printf (duperr, herr, pid);
-
-  /* Returning with procinfo == NULL.  Any open handles will be closed by the
-     destructor. */
+  /* cygheap_exec_info::record_children set the inheritance of the required
+     child handles so just copy them over... */
+  rd_proc_pipe = from.rd_proc_pipe;
+  hProcess = from.hProcess;
+  h = from.h;
+  /* ...and reset their inheritance. */
+  set_inheritance (false);
+  init (pid, PID_MAP_RW, h);
 }
 
 const char *
diff --git a/winsup/cygwin/pinfo.h b/winsup/cygwin/pinfo.h
index 8cf1bba..23f3083 100644
--- a/winsup/cygwin/pinfo.h
+++ b/winsup/cygwin/pinfo.h
@@ -139,6 +139,7 @@ public:
   HANDLE rd_proc_pipe;
   pinfo_minimal (): h (NULL), hProcess (NULL), rd_proc_pipe (NULL) {}
   void set_rd_proc_pipe (HANDLE& h) {rd_proc_pipe = h;}
+  void set_inheritance (bool);
   friend class pinfo;
 };
 
diff --git a/winsup/cygwin/release/3.1.0 b/winsup/cygwin/release/3.1.0
index fb0e372..23f752c 100644
--- a/winsup/cygwin/release/3.1.0
+++ b/winsup/cygwin/release/3.1.0
@@ -94,3 +94,6 @@ Bug Fixes
 
 - Make spawnvp, spawnvpe fail if the executable is not in $PATH.
   Addresses: https://cygwin.com/ml/cygwin/2019-10/msg00032.html
+
+- Fix parent/child relationship after parent dies.
+  Addresses: https://cygwin.com/ml/cygwin/2019-09/msg00263.html
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 91abb71..aff1ed6 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -949,6 +949,9 @@ cygheap_exec_info::record_children ()
     {
       children[nchildren].pid = procs[nchildren]->pid;
       children[nchildren].p = procs[nchildren];
+      /* Set inheritance of required child handles for reattach_children
+	 in the about-to-be-execed process. */
+      children[nchildren].p.set_inheritance (true);
     }
 }