Skip to content

Commit 7b2716b

Browse files
authored
Add handle ownership at creation time for Windows IPC (#89486)
1 parent 75ecb1a commit 7b2716b

File tree

2 files changed

+56
-2
lines changed

2 files changed

+56
-2
lines changed

src/native/eventpipe/ds-ipc-pal-namedpipe.c

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ ds_ipc_alloc (
132132

133133
instance->overlap.hEvent = INVALID_HANDLE_VALUE;
134134
instance->pipe = INVALID_HANDLE_VALUE;
135+
instance->owningPipe = INVALID_HANDLE_VALUE;
135136

136137
if (ipc_name) {
137138
characters_written = sprintf_s (
@@ -350,14 +351,27 @@ ds_ipc_listen (
350351

351352
EP_ASSERT (ipc->pipe == INVALID_HANDLE_VALUE);
352353

354+
DWORD creationFlags = PIPE_ACCESS_DUPLEX // read/write access
355+
| FILE_FLAG_OVERLAPPED; // async listening.
356+
357+
bool ensure_pipe_creation = ipc->owningPipe == INVALID_HANDLE_VALUE;
358+
if (ensure_pipe_creation)
359+
{
360+
// Fail if we can't own pipe. Other than manually iterating the DACL,
361+
// this is the only way to ensure ownership of the pipe via creation,
362+
// and by extension that it has the default DACL.
363+
// Otherwise, Windows treats this as a FIFO queue get-or-create
364+
// request and we might end up with DACLs set by other creators.
365+
creationFlags |= FILE_FLAG_FIRST_PIPE_INSTANCE;
366+
}
367+
353368
const uint32_t in_buffer_size = 16 * 1024;
354369
const uint32_t out_buffer_size = 16 * 1024;
355370

356371
DS_ENTER_BLOCKING_PAL_SECTION;
357372
ipc->pipe = CreateNamedPipeA (
358373
ipc->pipe_name, // pipe name
359-
PIPE_ACCESS_DUPLEX | // read/write access
360-
FILE_FLAG_OVERLAPPED, // async listening
374+
creationFlags,
361375
PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, // message type pipe, message-read and blocking mode
362376
PIPE_UNLIMITED_INSTANCES, // max. instances
363377
out_buffer_size, // output buffer size
@@ -372,6 +386,25 @@ ds_ipc_listen (
372386
ep_raise_error ();
373387
}
374388

389+
if (ensure_pipe_creation)
390+
{
391+
// Dupe the handle and hang it off the IPC to ensure EP ownership for process duration.
392+
bool createdSentinel = DuplicateHandle(
393+
GetCurrentProcess(),
394+
ipc->pipe,
395+
GetCurrentProcess(),
396+
&(ipc->owningPipe),
397+
0,
398+
FALSE,
399+
DUPLICATE_SAME_ACCESS);
400+
if (!createdSentinel)
401+
{
402+
if (callback)
403+
callback ("Failed to ownership sentinel.", GetLastError());
404+
ep_raise_error();
405+
}
406+
}
407+
375408
EP_ASSERT (ipc->overlap.hEvent == INVALID_HANDLE_VALUE);
376409

377410
ipc->overlap.hEvent = CreateEventW (NULL, true, false, NULL);
@@ -458,6 +491,9 @@ ds_ipc_accept (
458491
CloseHandle (ipc->overlap.hEvent);
459492
memset(&ipc->overlap, 0, sizeof(OVERLAPPED)); // clear the overlapped objects state
460493
ipc->overlap.hEvent = INVALID_HANDLE_VALUE;
494+
// We explicitly leave the ownership pipe handle untouched to root the IPC as long as the pipe has
495+
// been bound to our process.
496+
EP_ASSERT (ipc->owningPipe != INVALID_HANDLE_VALUE);
461497

462498
ep_raise_error_if_nok (ds_ipc_listen (ipc, callback));
463499

@@ -534,6 +570,21 @@ ds_ipc_close (
534570
{
535571
EP_ASSERT (ipc != NULL);
536572

573+
// Always clean this resource - even on shutdown - since
574+
// it roots resources accross embedding scenarios.
575+
if (ipc->owningPipe != INVALID_HANDLE_VALUE) {
576+
// Explicitly don't reset ownership if we fail to close.
577+
// It gives us a diagnostic crumble.
578+
if (CloseHandle(ipc->owningPipe) == TRUE) {
579+
ipc->owningPipe = INVALID_HANDLE_VALUE;
580+
}
581+
else {
582+
if (callback) {
583+
callback ("Failed to IPC ownership sentinel handle", GetLastError());
584+
}
585+
}
586+
}
587+
537588
// don't attempt cleanup on shutdown and let the OS handle it
538589
if (is_shutdown) {
539590
if (callback)

src/native/eventpipe/ds-ipc-pal-namedpipe.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ struct _DiagnosticsIpc_Internal {
2929
ep_char8_t pipe_name [DS_IPC_WIN32_MAX_NAMED_PIPE_LEN];
3030
OVERLAPPED overlap;
3131
HANDLE pipe;
32+
// This handle roots the ownership of the pipe from first listen until it
33+
// all sessions are closed.
34+
HANDLE owningPipe;
3235
bool is_listening;
3336
DiagnosticsIpcConnectionMode mode;
3437
};

0 commit comments

Comments
 (0)