From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 126664 invoked by alias); 3 Nov 2019 12:58:18 -0000 Mailing-List: contact cygwin-cvs-help@cygwin.com; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: cygwin-cvs-owner@cygwin.com Received: (qmail 126626 invoked by uid 9078); 3 Nov 2019 12:58:18 -0000 Date: Sun, 03 Nov 2019 12:58:00 -0000 Message-ID: <20191103125818.126624.qmail@sourceware.org> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Corinna Vinschen To: cygwin-cvs@sourceware.org Subject: [newlib-cygwin] Cygwin: fix process parent/child relationship after execve X-Act-Checkin: newlib-cygwin X-Git-Author: Corinna Vinschen X-Git-Refname: refs/heads/master X-Git-Oldrev: fa14f445ba1fb360fb5c4957798bf788345b0ef2 X-Git-Newrev: 57640bee75d18fac5ce6507f9a2c7a712ca750d8 X-SW-Source: 2019-q4/txt/msg00019.txt.bz2 https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=57640bee75d18fac5ce6507f9a2c7a712ca750d8 commit 57640bee75d18fac5ce6507f9a2c7a712ca750d8 Author: Corinna Vinschen 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 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); } }