[PATCH 17/21] Cygwin: FIFO: take ownership on exec
Ken Brown
kbrown@cornell.edu
Thu May 7 20:21:20 GMT 2020
If fixup_after_exec is called on a non-close-on-exec reader whose
parent is the owner, transfer ownership to the child. Otherwise the
parent's pipe handles will be closed before any other reader can
duplicate them.
To help with this, make the cancel_evt and thr_sync_evt handles
inheritable, so that the child can terminate the parent's
fifo_reader_thread (and the parent will update the shared fc_handler
list).
Add an optional argument 'from_exec' to update_my_handlers to simplify
its use in this case; no handle duplication is required.
---
winsup/cygwin/fhandler.h | 2 +-
winsup/cygwin/fhandler_fifo.cc | 151 +++++++++++++++++++++++----------
2 files changed, 107 insertions(+), 46 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 34b209f5d..1cd7d2b11 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1414,7 +1414,7 @@ class fhandler_fifo: public fhandler_base
{ return shmem->get_shared_fc_handler_committed (); }
void set_shared_fc_handler_committed (size_t n)
{ shmem->set_shared_fc_handler_committed (n); }
- int update_my_handlers ();
+ int update_my_handlers (bool from_exec = false);
int update_shared_handlers ();
public:
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 846115ad4..1c59bb3f4 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -104,13 +104,14 @@ sec_user_cloexec (bool cloexec, PSECURITY_ATTRIBUTES sa, PSID sid)
}
static HANDLE
-create_event ()
+create_event (bool inherit = false)
{
NTSTATUS status;
OBJECT_ATTRIBUTES attr;
HANDLE evt = NULL;
- InitializeObjectAttributes (&attr, NULL, 0, NULL, NULL);
+ InitializeObjectAttributes (&attr, NULL, inherit ? OBJ_INHERIT : 0,
+ NULL, NULL);
status = NtCreateEvent (&evt, EVENT_ALL_ACCESS, &attr,
NotificationEvent, FALSE);
if (!NT_SUCCESS (status))
@@ -353,47 +354,72 @@ fhandler_fifo::record_connection (fifo_client_handler& fc,
set_pipe_non_blocking (fc.h, true);
}
-/* Called from fifo_reader_thread_func with owner_lock in place. */
+/* Called from fifo_reader_thread_func with owner_lock in place, also
+ from fixup_after_exec with shared handles useable as they are. */
int
-fhandler_fifo::update_my_handlers ()
+fhandler_fifo::update_my_handlers (bool from_exec)
{
- close_all_handlers ();
- fifo_reader_id_t prev = get_prev_owner ();
- if (!prev)
+ if (from_exec)
{
- debug_printf ("No previous owner to copy handles from");
- return 0;
+ nhandlers = get_shared_nhandlers ();
+ if (nhandlers > shandlers)
+ {
+ int save = shandlers;
+ shandlers = nhandlers + 64;
+ void *temp = realloc (fc_handler,
+ shandlers * sizeof (fc_handler[0]));
+ if (!temp)
+ {
+ shandlers = save;
+ nhandlers = 0;
+ set_errno (ENOMEM);
+ return -1;
+ }
+ fc_handler = (fifo_client_handler *) temp;
+ }
+ memcpy (fc_handler, shared_fc_handler,
+ nhandlers * sizeof (fc_handler[0]));
}
- HANDLE prev_proc;
- if (prev.winpid == me.winpid)
- prev_proc = GetCurrentProcess ();
else
- prev_proc = OpenProcess (PROCESS_DUP_HANDLE, false, prev.winpid);
- if (!prev_proc)
{
- debug_printf ("Can't open process of previous owner, %E");
- __seterrno ();
- return -1;
- }
-
- for (int i = 0; i < get_shared_nhandlers (); i++)
- {
- /* Should never happen. */
- if (shared_fc_handler[i].state != fc_connected)
- continue;
- if (add_client_handler (false) < 0)
- api_fatal ("Can't add client handler, %E");
- fifo_client_handler &fc = fc_handler[nhandlers - 1];
- if (!DuplicateHandle (prev_proc, shared_fc_handler[i].h,
- GetCurrentProcess (), &fc.h, 0,
- !close_on_exec (), DUPLICATE_SAME_ACCESS))
+ close_all_handlers ();
+ fifo_reader_id_t prev = get_prev_owner ();
+ if (!prev)
+ {
+ debug_printf ("No previous owner to copy handles from");
+ return 0;
+ }
+ HANDLE prev_proc;
+ if (prev.winpid == me.winpid)
+ prev_proc = GetCurrentProcess ();
+ else
+ prev_proc = OpenProcess (PROCESS_DUP_HANDLE, false, prev.winpid);
+ if (!prev_proc)
{
- debug_printf ("Can't duplicate handle of previous owner, %E");
- --nhandlers;
+ debug_printf ("Can't open process of previous owner, %E");
__seterrno ();
return -1;
}
- fc.state = fc_connected;
+
+ for (int i = 0; i < get_shared_nhandlers (); i++)
+ {
+ /* Should never happen. */
+ if (shared_fc_handler[i].state != fc_connected)
+ continue;
+ if (add_client_handler (false) < 0)
+ api_fatal ("Can't add client handler, %E");
+ fifo_client_handler &fc = fc_handler[nhandlers - 1];
+ if (!DuplicateHandle (prev_proc, shared_fc_handler[i].h,
+ GetCurrentProcess (), &fc.h, 0,
+ !close_on_exec (), DUPLICATE_SAME_ACCESS))
+ {
+ debug_printf ("Can't duplicate handle of previous owner, %E");
+ --nhandlers;
+ __seterrno ();
+ return -1;
+ }
+ fc.state = fc_connected;
+ }
}
return 0;
}
@@ -771,9 +797,9 @@ fhandler_fifo::open (int flags, mode_t)
if (create_shared_fc_handler () < 0)
goto err_close_shmem;
inc_nreaders ();
- if (!(cancel_evt = create_event ()))
+ if (!(cancel_evt = create_event (true)))
goto err_dec_nreaders;
- if (!(thr_sync_evt = create_event ()))
+ if (!(thr_sync_evt = create_event (true)))
goto err_close_cancel_evt;
me.winpid = GetCurrentProcessId ();
me.fh = this;
@@ -1338,9 +1364,9 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
}
if (fhf->reopen_shared_fc_handler () < 0)
goto err_close_shared_fc_hdl;
- if (!(fhf->cancel_evt = create_event ()))
+ if (!(fhf->cancel_evt = create_event (true)))
goto err_close_shared_fc_handler;
- if (!(fhf->thr_sync_evt = create_event ()))
+ if (!(fhf->thr_sync_evt = create_event (true)))
goto err_close_cancel_evt;
inc_nreaders ();
fhf->me.fh = fhf;
@@ -1389,9 +1415,17 @@ fhandler_fifo::fixup_after_fork (HANDLE parent)
/* Prevent a later attempt to close the non-inherited
pipe-instance handles copied from the parent. */
nhandlers = 0;
- if (!(cancel_evt = create_event ()))
+ else
+ {
+ /* Close inherited handles needed only by exec. */
+ if (cancel_evt)
+ NtClose (cancel_evt);
+ if (thr_sync_evt)
+ NtClose (thr_sync_evt);
+ }
+ if (!(cancel_evt = create_event (true)))
api_fatal ("Can't create reader thread cancel event during fork, %E");
- if (!(thr_sync_evt = create_event ()))
+ if (!(thr_sync_evt = create_event (true)))
api_fatal ("Can't create reader thread sync event during fork, %E");
inc_nreaders ();
me.winpid = GetCurrentProcessId ();
@@ -1414,10 +1448,32 @@ fhandler_fifo::fixup_after_exec ()
api_fatal ("Can't reopen shared fc_handler memory during exec, %E");
fc_handler = NULL;
nhandlers = shandlers = 0;
+
+ /* Cancel parent's reader thread */
+ if (cancel_evt)
+ SetEvent (cancel_evt);
+ if (thr_sync_evt)
+ WaitForSingleObject (thr_sync_evt, INFINITE);
+
+ /* Take ownership if parent is owner. */
+ fifo_reader_id_t parent_fr = me;
me.winpid = GetCurrentProcessId ();
- if (!(cancel_evt = create_event ()))
+ owner_lock ();
+ if (get_owner () == parent_fr)
+ {
+ set_owner (me);
+ if (update_my_handlers (true) < 0)
+ api_fatal ("Can't update my handlers, %E");
+ }
+ owner_unlock ();
+ /* Close inherited cancel_evt and thr_sync_evt. */
+ if (cancel_evt)
+ NtClose (cancel_evt);
+ if (thr_sync_evt)
+ NtClose (thr_sync_evt);
+ if (!(cancel_evt = create_event (true)))
api_fatal ("Can't create reader thread cancel event during exec, %E");
- if (!(thr_sync_evt = create_event ()))
+ if (!(thr_sync_evt = create_event (true)))
api_fatal ("Can't create reader thread sync event during exec, %E");
/* At this moment we're a new reader. The count will be
decremented when the parent closes. */
@@ -1433,8 +1489,13 @@ fhandler_fifo::set_close_on_exec (bool val)
set_no_inheritance (read_ready, val);
set_no_inheritance (write_ready, val);
set_no_inheritance (writer_opening, val);
- fifo_client_lock ();
- for (int i = 0; i < nhandlers; i++)
- set_no_inheritance (fc_handler[i].h, val);
- fifo_client_unlock ();
+ if (reader)
+ {
+ set_no_inheritance (cancel_evt, val);
+ set_no_inheritance (thr_sync_evt, val);
+ fifo_client_lock ();
+ for (int i = 0; i < nhandlers; i++)
+ set_no_inheritance (fc_handler[i].h, val);
+ fifo_client_unlock ();
+ }
}
--
2.21.0
More information about the Cygwin-patches
mailing list