On Tue, 5 Mar 2024 11:14:46 +0100 Corinna Vinschen wrote: > On Mar 5 09:06, Takashi Yano wrote: > > On Mon, 4 Mar 2024 18:38:07 +0100 > > Corinna Vinschen wrote: > > > On Mar 4 16:45, ASSI wrote: > > > > Corinna Vinschen writes: > > > > > Right you are. We always said that independent Cygwin installations > > > > > are supposed to *stay* independent. > > > > > > > > > > Keep in mind that they don't share the same shared objects in the > > > > > native > > > > > NT namespace and they don't know of each other. It's not only the > > > > > process table but also in-use FIFO stuff, pty info, etc. > > > > > > > > What I was getting at is that a process not showing up in the process > > > > list in one Cygwin installation doesn't automatically mean it's a native > > > > Windows process, it could be a process started by an independent Cygwin > > > > installation. So this way of checking for "native" Windows processes > > > > may or may not do what was originally intended. > > > > > > But that was my point. A "foreign" Cygwin process from another > > > installation is not a Cygwin process. Lots of interoperability > > > just won't work, so it's basically a native process. > > > > Actually, I think query_hdl can be retrieved from the process > > from another installation of cygwin using NtQueryInformationProcess() > > with ProcessHandleInformation. However, I cannot imagne the case > > that the pipe is made by one cygwin installation but the reader > > process is from another installation of cygwin. > > > > BTW, what about v2 patch itself? > > It does the job with less code and less memory, which is good. > I would change the comment > > stop to try to get query_hdl for non-cygwin apps > > to something like > > don't try to fetch query_hdl from non-cygwin apps > > "stop trying" is a bit of a back-reference to the old code, which > is not necessary, I think.
I'll submit v3 patch. Please review. > This doesn't affect your patch, but while looking into this, what > strikes me as weird is that fhandler_pipe::temporary_query_hdl() calls > NtQueryObject() and assembles the pipe name via swscanf() every time it > is called. > > Wouldn't it make sense to store the name in the fhandler's > path_conv::wide_path/uni_path at creation time instead? > The wide_path member is not used at all in pipes, ostensibly. Is the patch attached as you intended? -- Takashi Yano <takashi.y...@nifty.ne.jp>
From 998027fe7a7a56af9507341266c9fcadee66beb9 Mon Sep 17 00:00:00 2001 From: Takashi Yano <takashi.y...@nifty.ne.jp> Date: Tue, 5 Mar 2024 23:34:21 +0900 Subject: [PATCH] Cygwin: pipe: Simplify chhecking procedure of query_hdl. This patch eliminates verbose NtQueryObject() calls in the procedure to get query_hdl by storing pipe name into fhandler_base::pc when the pipe is created. fhandler_pipe::temporary_query_hdl() uses the storedpipe name rather than the name retrieved by NtQueryObject(). Suggested-by: Corinna Vinschen <cori...@vinschen.de> Signed-off-by: Takashi Yano <takashi.y...@nifty.ne.jp> --- winsup/cygwin/fhandler/pipe.cc | 39 ++++++++++++------------- winsup/cygwin/local_includes/fhandler.h | 3 -- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc index c877d89d7..0611dd1c3 100644 --- a/winsup/cygwin/fhandler/pipe.cc +++ b/winsup/cygwin/fhandler/pipe.cc @@ -93,6 +93,19 @@ fhandler_pipe::init (HANDLE f, DWORD a, mode_t mode, int64_t uniq_id) even with FILE_SYNCHRONOUS_IO_NONALERT. */ set_pipe_non_blocking (get_device () == FH_PIPER ? true : is_nonblocking ()); + + /* Store pipe name to path_conv pc for query_hdl check */ + if (get_dev () == FH_PIPEW) + { + ULONG len; + tmp_pathbuf tp; + OBJECT_NAME_INFORMATION *ntfn = (OBJECT_NAME_INFORMATION *) tp.w_get (); + NTSTATUS status = NtQueryObject (f, ObjectNameInformation, ntfn, + 65536, &len); + if (NT_SUCCESS (status) && ntfn->Name.Buffer) + pc.set_nt_native_path (&ntfn->Name); + } + return 1; } @@ -1149,6 +1162,9 @@ fhandler_pipe::temporary_query_hdl () tmp_pathbuf tp; OBJECT_NAME_INFORMATION *ntfn = (OBJECT_NAME_INFORMATION *) tp.w_get (); + UNICODE_STRING *name = pc.get_nt_native_path (NULL); + name->Buffer[name->Length / sizeof (WCHAR)] = L'\0'; + /* Try process handle opened and pipe handle value cached first in order to reduce overhead. */ if (query_hdl_proc && query_hdl_value) @@ -1161,14 +1177,7 @@ fhandler_pipe::temporary_query_hdl () 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) + if (RtlEqualUnicodeString (name, &ntfn->Name, FALSE)) return h; hdl_err: CloseHandle (h); @@ -1178,19 +1187,9 @@ cache_err: query_hdl_value = NULL; } - status = NtQueryObject (get_handle (), ObjectNameInformation, ntfn, - 65536, &len); - if (!NT_SUCCESS (status) || !ntfn->Name.Buffer) + if (name->Length == 0 || name->Buffer == NULL) 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? */ - - return get_query_hdl_per_process (name, ntfn); /* Since Win8 */ + return get_query_hdl_per_process (name->Buffer, ntfn); /* Since Win8 */ } HANDLE diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h index 6ddf37370..028eb49d0 100644 --- a/winsup/cygwin/local_includes/fhandler.h +++ b/winsup/cygwin/local_includes/fhandler.h @@ -1216,9 +1216,6 @@ private: HANDLE query_hdl_proc; HANDLE query_hdl_value; HANDLE query_hdl_close_req_evt; - uint64_t pipename_key; - DWORD pipename_pid; - LONG pipename_id; void release_select_sem (const char *); HANDLE get_query_hdl_per_process (WCHAR *, OBJECT_NAME_INFORMATION *); public: -- 2.43.0