From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2155) id 1FC1C3857818; Tue, 14 Sep 2021 15:05:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1FC1C3857818 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Corinna Vinschen To: cygwin-cvs@sourceware.org Subject: [newlib-cygwin] Revert "Cygwin: pipe: Revert to create() rather than nt_create()." X-Act-Checkin: newlib-cygwin X-Git-Author: Ken Brown X-Git-Refname: refs/heads/master X-Git-Oldrev: cf3a7a9132fbc3eae466709c63b68f29f4b91ddc X-Git-Newrev: 24b7a74b9481cca9772735d2296e6cec5e7cb204 Message-Id: <20210914150519.1FC1C3857818@sourceware.org> Date: Tue, 14 Sep 2021 15:05:19 +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, 14 Sep 2021 15:05:19 -0000 https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=24b7a74b9481cca9772735d2296e6cec5e7cb204 commit 24b7a74b9481cca9772735d2296e6cec5e7cb204 Author: Ken Brown Date: Mon Aug 30 09:53:06 2021 -0400 Revert "Cygwin: pipe: Revert to create() rather than nt_create()." This reverts commit 5a7a0d34c74a55aa1e76644e61bf4889051cb640. Diff: --- winsup/cygwin/fhandler_pipe.cc | 192 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 171 insertions(+), 21 deletions(-) diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc index a6ece17a6..2dec0a848 100644 --- a/winsup/cygwin/fhandler_pipe.cc +++ b/winsup/cygwin/fhandler_pipe.cc @@ -463,13 +463,13 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w, if (name) len += __small_sprintf (pipename + len, "%s", name); - open_mode |= PIPE_ACCESS_OUTBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE; + open_mode |= PIPE_ACCESS_INBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE; /* Retry CreateNamedPipe as long as the pipe name is in use. Retrying will probably never be necessary, but we want to be as robust as possible. */ DWORD err = 0; - while (w && !*w) + while (r && !*r) { static volatile ULONG pipe_unique_id; if (!name) @@ -498,12 +498,12 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w, definitely required for pty handling since fhandler_pty_master writes to the pipe in chunks, terminated by newline when CANON mode is specified. */ - *w = CreateNamedPipe (pipename, open_mode, pipe_mode, 1, psize, + *r = CreateNamedPipe (pipename, open_mode, pipe_mode, 1, psize, psize, NMPWAIT_USE_DEFAULT_WAIT, sa_ptr); - if (*w != INVALID_HANDLE_VALUE) + if (*r != INVALID_HANDLE_VALUE) { - debug_printf ("pipe write handle %p", *w); + debug_printf ("pipe read handle %p", *r); err = 0; break; } @@ -516,14 +516,14 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w, Pick a new name and retry. */ debug_printf ("pipe busy", !name ? ", retrying" : ""); if (!name) - *w = NULL; + *r = NULL; break; case ERROR_ACCESS_DENIED: /* The pipe is already open with incompatible parameters. Pick a new name and retry. */ debug_printf ("pipe access denied%s", !name ? ", retrying" : ""); if (!name) - *w = NULL; + *r = NULL; break; default: { @@ -535,42 +535,60 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w, if (err) { - *w = NULL; + *r = NULL; return err; } - if (!r) - debug_printf ("pipe read handle NULL"); + if (!w) + debug_printf ("pipe write handle NULL"); else { debug_printf ("CreateFile: name %s", pipename); - /* Open the named pipe for reading. - Be sure to permit FILE_WRITE_ATTRIBUTES access. */ - DWORD access = GENERIC_READ | FILE_WRITE_ATTRIBUTES; + /* Open the named pipe for writing. + Be sure to permit FILE_READ_ATTRIBUTES access. */ + DWORD access = GENERIC_WRITE | FILE_READ_ATTRIBUTES; if ((open_mode & PIPE_ACCESS_DUPLEX) == PIPE_ACCESS_DUPLEX) - access |= GENERIC_WRITE | FILE_READ_ATTRIBUTES; - *r = CreateFile (pipename, access, 0, sa_ptr, OPEN_EXISTING, + access |= GENERIC_READ | FILE_WRITE_ATTRIBUTES; + *w = CreateFile (pipename, access, 0, sa_ptr, OPEN_EXISTING, open_mode & FILE_FLAG_OVERLAPPED, 0); - if (!*r || *r == INVALID_HANDLE_VALUE) + if (!*w || *w == INVALID_HANDLE_VALUE) { /* Failure. */ DWORD err = GetLastError (); debug_printf ("CreateFile failed, r %p, %E", r); - if (w) - CloseHandle (*w); - *r = NULL; + if (r) + CloseHandle (*r); + *w = NULL; return err; } - debug_printf ("pipe read handle %p", *r); + debug_printf ("pipe write handle %p", *w); } /* Success. */ return 0; } +/* 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 + PIPE_ACCESS_DUPLEX, and it doesn't seem possible to add that access + right. This causes set_pipe_non_blocking to fail. + + To fix this we will define a helper function 'nt_create' that is + similar to the above fhandler_pipe::create but uses + NtCreateNamedPipeFile instead of CreateNamedPipe; this gives more + flexibility in setting the access rights. We will use this helper + function in the version of fhandler_pipe::create below, which + suffices for all of our uses of set_pipe_non_blocking. For + simplicity, nt_create will omit the 'open_mode' and 'name' + parameters, which aren't needed for our purposes. */ + +static int nt_create (LPSECURITY_ATTRIBUTES, PHANDLE, PHANDLE, DWORD, + int64_t *); + int fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode) { @@ -579,7 +597,7 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode) int res = -1; int64_t unique_id; - int ret = create (sa, &r, &w, psize, NULL, 0, &unique_id); + int ret = nt_create (sa, &r, &w, psize, &unique_id); if (ret) __seterrno_from_win_error (ret); else if ((fhs[0] = (fhandler_pipe *) build_fh_dev (*piper_dev)) == NULL) @@ -606,6 +624,138 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode) return res; } +static int +nt_create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w, + DWORD psize, int64_t *unique_id) +{ + NTSTATUS status; + HANDLE npfsh; + ACCESS_MASK access; + OBJECT_ATTRIBUTES attr; + IO_STATUS_BLOCK io; + LARGE_INTEGER timeout; + + /* Default to error. */ + if (r) + *r = NULL; + if (w) + *w = NULL; + + status = fhandler_base::npfs_handle (npfsh); + if (!NT_SUCCESS (status)) + { + __seterrno_from_nt_status (status); + return GetLastError (); + } + + /* Ensure that there is enough pipe buffer space for atomic writes. */ + if (!psize) + psize = DEFAULT_PIPEBUFSIZE; + + UNICODE_STRING pipename; + WCHAR pipename_buf[MAX_PATH]; + size_t len = __small_swprintf (pipename_buf, L"%S-%u-", + &cygheap->installation_key, + GetCurrentProcessId ()); + + access = GENERIC_READ | FILE_WRITE_ATTRIBUTES; + + ULONG pipe_type = pipe_byte ? FILE_PIPE_BYTE_STREAM_TYPE + : FILE_PIPE_MESSAGE_TYPE; + + /* Retry NtCreateNamedPipeFile as long as the pipe name is in use. + Retrying will probably never be necessary, but we want + to be as robust as possible. */ + DWORD err = 0; + while (r && !*r) + { + static volatile ULONG pipe_unique_id; + LONG id = InterlockedIncrement ((LONG *) &pipe_unique_id); + __small_swprintf (pipename_buf + len, L"pipe-nt-%p", id); + if (unique_id) + *unique_id = ((int64_t) id << 32 | GetCurrentProcessId ()); + + debug_printf ("name %W, size %u, mode %s", pipename_buf, psize, + (pipe_type & FILE_PIPE_MESSAGE_TYPE) + ? "PIPE_TYPE_MESSAGE" : "PIPE_TYPE_BYTE"); + + RtlInitUnicodeString (&pipename, pipename_buf); + + InitializeObjectAttributes (&attr, &pipename, + sa_ptr->bInheritHandle ? OBJ_INHERIT : 0, + npfsh, sa_ptr->lpSecurityDescriptor); + + timeout.QuadPart = -500000; + status = NtCreateNamedPipeFile (r, access, &attr, &io, + FILE_SHARE_READ | FILE_SHARE_WRITE, + FILE_CREATE, 0, pipe_type, + FILE_PIPE_BYTE_STREAM_MODE, + 0, 1, psize, psize, &timeout); + + if (NT_SUCCESS (status)) + { + debug_printf ("pipe read handle %p", *r); + err = 0; + break; + } + + switch (status) + { + case STATUS_PIPE_BUSY: + case STATUS_INSTANCE_NOT_AVAILABLE: + case STATUS_PIPE_NOT_AVAILABLE: + /* The pipe is already open with compatible parameters. + Pick a new name and retry. */ + debug_printf ("pipe busy, retrying"); + *r = NULL; + break; + case STATUS_ACCESS_DENIED: + /* The pipe is already open with incompatible parameters. + Pick a new name and retry. */ + debug_printf ("pipe access denied, retrying"); + *r = NULL; + break; + default: + { + __seterrno_from_nt_status (status); + err = GetLastError (); + debug_printf ("failed, %E"); + *r = INVALID_HANDLE_VALUE; + } + } + } + + if (err) + { + *r = NULL; + return err; + } + + if (!w) + debug_printf ("pipe write handle NULL"); + else + { + debug_printf ("NtOpenFile: name %S", &pipename); + + access = GENERIC_WRITE | FILE_READ_ATTRIBUTES; + status = NtOpenFile (w, access, &attr, &io, 0, 0); + if (!NT_SUCCESS (status)) + { + DWORD err = GetLastError (); + debug_printf ("NtOpenFile failed, r %p, %E", r); + if (r) + CloseHandle (*r); + *w = NULL; + return err; + } + + debug_printf ("pipe write handle %p", *w); + } + + /* Success. */ + return 0; +} + int fhandler_pipe::ioctl (unsigned int cmd, void *p) {