From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2210) id 9EFA8385841C; Tue, 21 Sep 2021 18:51:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9EFA8385841C Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Ken Brown To: cygwin-cvs@sourceware.org Subject: [newlib-cygwin] Cygwin: pipe: Introduce temporary query_hdl. X-Act-Checkin: newlib-cygwin X-Git-Author: Takashi Yano X-Git-Refname: refs/heads/master X-Git-Oldrev: 643db9ec9e13724ff9cb7254bbfdc292463627a5 X-Git-Newrev: b531d6b06eeb9abad9ba3e41171e23a94c593b0d Message-Id: <20210921185112.9EFA8385841C@sourceware.org> Date: Tue, 21 Sep 2021 18:51:12 +0000 (GMT) X-BeenThere: cygwin-cvs@cygwin.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Cygwin core component git logs List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Sep 2021 18:51:12 -0000 https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=b531d6b06eeb9abad9ba3e41171e23a94c593b0d commit b531d6b06eeb9abad9ba3e41171e23a94c593b0d Author: Takashi Yano Date: Tue Sep 21 08:02:43 2021 +0900 Cygwin: pipe: Introduce temporary query_hdl. - The commit f79a4611 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 f79a4611 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. Diff: --- winsup/cygwin/fhandler.h | 6 ++ winsup/cygwin/fhandler_pipe.cc | 136 +++++++++++++++++++++++++++++++++++++++-- winsup/cygwin/ntdll.h | 17 ++++++ winsup/cygwin/select.cc | 8 ++- 4 files changed, 161 insertions(+), 6 deletions(-) diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h index 3471e95b9..0061d4830 100644 --- a/winsup/cygwin/fhandler.h +++ b/winsup/cygwin/fhandler.h @@ -1191,6 +1191,11 @@ private: 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 (); @@ -1249,6 +1254,7 @@ public: } } bool reader_closed (); + HANDLE temporary_query_hdl (); }; #define CYGWIN_FIFO_PIPE_NAME_LEN 47 diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc index 6b41a755f..78e2f90d8 100644 --- a/winsup/cygwin/fhandler_pipe.cc +++ b/winsup/cygwin/fhandler_pipe.cc @@ -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. */ @@ -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: @@ -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) @@ -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; } @@ -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 @@ -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; @@ -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: @@ -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; @@ -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; +} diff --git a/winsup/cygwin/ntdll.h b/winsup/cygwin/ntdll.h index 4504bdf6d..e8c3c45c5 100644 --- a/winsup/cygwin/ntdll.h +++ b/winsup/cygwin/ntdll.h @@ -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 diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc index 33c0c0bb0..a2868abd0 100644 --- a/winsup/cygwin/select.cc +++ b/winsup/cygwin/select.cc @@ -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; }