From 57640bee75d18fac5ce6507f9a2c7a712ca750d8 Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <corinna@vinschen.de>
Date: Sat, 2 Nov 2019 12:49:15 +0100
Subject: [PATCH] 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>
---
 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 35c1ffe25..800adbff1 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 8cf1bba2e..23f308360 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 fb0e37215..23f752ca7 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 91abb717c..aff1ed61b 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);
     }
 }