Skip to content

Commit

Permalink
Cygwin: pipe: Introduce temporary query_hdl.
Browse files Browse the repository at this point in the history
- The commit f79a461 introduced query_hdl, which is the read pipe
  handle kept in the write pipe instance in order to determine if
  the pipe is ready to write in select().  This implementation has
  a potential risk that the write side fails to detect the closure
  of the read side if more than one writer exists and one of them
  is a non-cygwin process.

  With this patch, the strategy of commit f79a461 is used only if
  the process is running as a service.  For a normal process,
  instead of keeping query_hdl in the write pipe instance, it is
  retrieved temporarily when select() is called.  Actually, we
  want to use tenporary query_hdl for all processes, however, it
  does not work for service processes due to OpenProcess()
  failure.
  • Loading branch information
tyan0 authored and kbrow1i committed Sep 21, 2021
1 parent 643db9e commit b531d6b
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 6 deletions.
6 changes: 6 additions & 0 deletions winsup/cygwin/fhandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1191,6 +1191,11 @@ class fhandler_pipe: public fhandler_pipe_fifo
pid_t popen_pid;
HANDLE query_hdl;
HANDLE hdl_cnt_mtx;
HANDLE query_hdl_proc;
HANDLE query_hdl_value;
uint64_t pipename_key;
DWORD pipename_pid;
LONG pipename_id;
void release_select_sem (const char *);
public:
fhandler_pipe ();
Expand Down Expand Up @@ -1249,6 +1254,7 @@ class fhandler_pipe: public fhandler_pipe_fifo
}
}
bool reader_closed ();
HANDLE temporary_query_hdl ();
};

#define CYGWIN_FIFO_PIPE_NAME_LEN 47
Expand Down
136 changes: 131 additions & 5 deletions winsup/cygwin/fhandler_pipe.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ details. */
#include "cygheap.h"
#include "pinfo.h"
#include "shared_info.h"
#include "tls_pbuf.h"

/* This is only to be used for writing. When reading,
STATUS_PIPE_EMPTY simply means there's no data to be read. */
Expand Down Expand Up @@ -220,8 +221,6 @@ fhandler_pipe::open_setup (int flags)
goto err_close_read_mtx;
}
}
if (get_dev () == FH_PIPEW && !query_hdl)
set_pipe_non_blocking (is_nonblocking ());
return true;

err_close_read_mtx:
Expand Down Expand Up @@ -267,7 +266,7 @@ fhandler_pipe::release_select_sem (const char *from)
- get_obj_handle_count (read_mtx);
else /* Number of select() call */
n_release = get_obj_handle_count (select_sem)
- get_obj_handle_count (query_hdl);
- get_obj_handle_count (hdl_cnt_mtx);
debug_printf("%s(%s) release %d", from,
get_dev () == FH_PIPER ? "PIPER" : "PIPEW", n_release);
if (n_release)
Expand Down Expand Up @@ -667,6 +666,8 @@ fhandler_pipe::close ()
int ret = fhandler_base::close ();
ReleaseMutex (hdl_cnt_mtx);
CloseHandle (hdl_cnt_mtx);
if (query_hdl_proc)
CloseHandle (query_hdl_proc);
return ret;
}

Expand Down Expand Up @@ -820,6 +821,13 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w,
return 0;
}

inline static bool
is_running_as_service (void)
{
return check_token_membership (well_known_service_sid)
|| cygheap->user.saved_sid () == well_known_system_sid;
}

/* The next version of fhandler_pipe::create used to call the previous
version. But the read handle created by the latter doesn't have
FILE_WRITE_ATTRIBUTES access unless the pipe is created with
Expand Down Expand Up @@ -874,7 +882,8 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode)
0, sa->bInheritHandle, DUPLICATE_SAME_ACCESS))
goto err_close_select_sem0;

if (!DuplicateHandle (GetCurrentProcess (), r,
if (is_running_as_service () &&
!DuplicateHandle (GetCurrentProcess (), r,
GetCurrentProcess (), &fhs[1]->query_hdl,
FILE_READ_DATA, sa->bInheritHandle, 0))
goto err_close_select_sem1;
Expand All @@ -893,7 +902,8 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode)
err_close_hdl_cnt_mtx0:
CloseHandle (fhs[0]->hdl_cnt_mtx);
err_close_query_hdl:
CloseHandle (fhs[1]->query_hdl);
if (fhs[1]->query_hdl)
CloseHandle (fhs[1]->query_hdl);
err_close_select_sem1:
CloseHandle (fhs[1]->select_sem);
err_close_select_sem0:
Expand Down Expand Up @@ -946,6 +956,7 @@ nt_create (LPSECURITY_ATTRIBUTES sa_ptr, HANDLE &r, HANDLE &w,
GetCurrentProcessId ());

access = GENERIC_READ | FILE_WRITE_ATTRIBUTES | SYNCHRONIZE;
access |= FILE_WRITE_EA; /* Add this right as a marker of cygwin read pipe */

ULONG pipe_type = pipe_byte ? FILE_PIPE_BYTE_STREAM_TYPE
: FILE_PIPE_MESSAGE_TYPE;
Expand Down Expand Up @@ -1112,3 +1123,118 @@ fhandler_pipe::fstatvfs (struct statvfs *sfs)
set_errno (EBADF);
return -1;
}

HANDLE
fhandler_pipe::temporary_query_hdl ()
{
if (get_dev () != FH_PIPEW)
return NULL;

ULONG len;
NTSTATUS status;
tmp_pathbuf tp;
OBJECT_NAME_INFORMATION *ntfn = (OBJECT_NAME_INFORMATION *) tp.w_get ();

/* Try process handle opened and pipe handle value cached first
in order to reduce overhead. */
if (query_hdl_proc && query_hdl_value)
{
HANDLE h;
if (!DuplicateHandle (query_hdl_proc, query_hdl_value,
GetCurrentProcess (), &h, FILE_READ_DATA, 0, 0))
goto cache_err;
/* Check name */
status = NtQueryObject (h, ObjectNameInformation, ntfn, 65536, &len);
if (!NT_SUCCESS (status) || !ntfn->Name.Buffer)
goto hdl_err;
ntfn->Name.Buffer[ntfn->Name.Length / sizeof (WCHAR)] = L'\0';
uint64_t key;
DWORD pid;
LONG id;
if (swscanf (ntfn->Name.Buffer,
L"\\Device\\NamedPipe\\%llx-%u-pipe-nt-0x%x",
&key, &pid, &id) == 3 &&
key == pipename_key && pid == pipename_pid && id == pipename_id)
return h;
hdl_err:
CloseHandle (h);
cache_err:
CloseHandle (query_hdl_proc);
query_hdl_proc = NULL;
query_hdl_value = NULL;
}

status = NtQueryObject (get_handle (), ObjectNameInformation, ntfn,
65536, &len);
if (!NT_SUCCESS (status) || !ntfn->Name.Buffer)
return NULL; /* Non cygwin pipe? */
WCHAR name[MAX_PATH];
int namelen = min (ntfn->Name.Length / sizeof (WCHAR), MAX_PATH-1);
memcpy (name, ntfn->Name.Buffer, namelen * sizeof (WCHAR));
name[namelen] = L'\0';
if (swscanf (name, L"\\Device\\NamedPipe\\%llx-%u-pipe-nt-0x%x",
&pipename_key, &pipename_pid, &pipename_id) != 3)
return NULL; /* Non cygwin pipe? */

SIZE_T n_handle = 65536;
PSYSTEM_HANDLE_INFORMATION shi;
do
{
SIZE_T nbytes =
sizeof (ULONG) + n_handle * sizeof (SYSTEM_HANDLE_TABLE_ENTRY_INFO);
shi = (PSYSTEM_HANDLE_INFORMATION) HeapAlloc (GetProcessHeap (),
0, nbytes);
if (!shi)
return NULL;
status = NtQuerySystemInformation (SystemHandleInformation,
shi, nbytes, NULL);
if (NT_SUCCESS (status))
break;
HeapFree (GetProcessHeap (), 0, shi);
n_handle *= 2;
}
while (n_handle < (1L<<20));
if (!NT_SUCCESS (status))
return NULL;

HANDLE qh = NULL;
for (LONG i = (LONG) shi->NumberOfHandles - 1; i >= 0; i--)
{
/* Check for the peculiarity of cygwin read pipe */
DWORD access = FILE_READ_DATA | FILE_READ_EA | FILE_WRITE_EA /* marker */
| FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES
| READ_CONTROL | SYNCHRONIZE;
if (shi->Handles[i].GrantedAccess != access)
continue;

/* Retrieve handle */
HANDLE proc = OpenProcess (PROCESS_DUP_HANDLE, 0,
shi->Handles[i].UniqueProcessId);
if (!proc)
continue;
HANDLE h = (HANDLE)(intptr_t) shi->Handles[i].HandleValue;
BOOL res = DuplicateHandle (proc, h, GetCurrentProcess (), &h,
FILE_READ_DATA, 0, 0);
if (!res)
goto close_proc;

/* Check object name */
status = NtQueryObject (h, ObjectNameInformation, ntfn, 65536, &len);
if (!NT_SUCCESS (status) || !ntfn->Name.Buffer)
goto close_handle;
ntfn->Name.Buffer[ntfn->Name.Length / sizeof (WCHAR)] = L'\0';
if (wcscmp (name, ntfn->Name.Buffer) == 0)
{
query_hdl_proc = proc;
query_hdl_value = (HANDLE)(intptr_t) shi->Handles[i].HandleValue;
qh = h;
break;
}
close_handle:
CloseHandle (h);
close_proc:
CloseHandle (proc);
}
HeapFree (GetProcessHeap (), 0, shi);
return qh;
}
17 changes: 17 additions & 0 deletions winsup/cygwin/ntdll.h
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,23 @@ typedef struct _SYSTEM_PROCESSOR_PERFORMANCE_INFORMATION
ULONG InterruptCount;
} SYSTEM_PROCESSOR_PERFORMANCE_INFORMATION, *PSYSTEM_PROCESSOR_PERFORMANCE_INFORMATION;

typedef struct _SYSTEM_HANDLE_TABLE_ENTRY_INFO
{
USHORT UniqueProcessId;
USHORT CreatorBackTraceIndex;
UCHAR ObjectTypeIndex;
UCHAR HandleAttributes;
USHORT HandleValue;
PVOID Object;
ULONG GrantedAccess;
} SYSTEM_HANDLE_TABLE_ENTRY_INFO, *PSYSTEM_HANDLE_TABLE_ENTRY_INFO;

typedef struct _SYSTEM_HANDLE_INFORMATION
{
ULONG NumberOfHandles;
SYSTEM_HANDLE_TABLE_ENTRY_INFO Handles[1];
} SYSTEM_HANDLE_INFORMATION, *PSYSTEM_HANDLE_INFORMATION;

typedef LONG KPRIORITY;

typedef struct _VM_COUNTERS
Expand Down
8 changes: 7 additions & 1 deletion winsup/cygwin/select.cc
Original file line number Diff line number Diff line change
Expand Up @@ -641,10 +641,16 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, bool writing)
if (fh->get_device () == FH_PIPEW && fpli.WriteQuotaAvailable == 0)
{
HANDLE query_hdl = ((fhandler_pipe *) fh)->get_query_handle ();
if (!query_hdl)
query_hdl = ((fhandler_pipe *) fh)->temporary_query_hdl ();
if (!query_hdl)
return 1; /* We cannot know actual write pipe space. */
DWORD nbytes_in_pipe;
if (!PeekNamedPipe (query_hdl, NULL, 0, NULL, &nbytes_in_pipe, NULL))
BOOL res =
PeekNamedPipe (query_hdl, NULL, 0, NULL, &nbytes_in_pipe, NULL);
if (!((fhandler_pipe *) fh)->get_query_handle ())
CloseHandle (query_hdl); /* Close temporary query_hdl */
if (!res)
return 1;
fpli.WriteQuotaAvailable = fpli.InboundQuota - nbytes_in_pipe;
}
Expand Down

0 comments on commit b531d6b

Please sign in to comment.