[PATCH draft 4/6] Cygwin: fhandler_pipe: fix permission problem
Ken Brown
kbrown@cornell.edu
Sun May 26 15:10:00 GMT 2019
The read handles of pipes created by CreateNamedPipe don't have
FILE_WRITE_ATTRIBUTES access unless the pipe is created with
PIPE_ACCESS_DUPLEX. This causes set_pipe_non_blocking to fail on such
handles. To fix this, add a helper function nt_create, which uses
NtCreateNamedPipeFile instead of CreateNamedPipe and gives us more
flexibility in setting access rights.
Use this helper function in the version of fhandler_pipe::create that
underlies the pipe and pipe2 system calls.
For convenience, also add a static member function
fhandler_pipe::npfs_handle identical to those used by fhandler_fifo
and fhandler_socket_unix.
---
winsup/cygwin/fhandler.h | 1 +
winsup/cygwin/fhandler_pipe.cc | 181 ++++++++++++++++++++++++++++++++-
2 files changed, 181 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 5ae262c5c..7aa01d101 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1216,6 +1216,7 @@ public:
int __reg3 fadvise (off_t, off_t, int);
int __reg3 ftruncate (off_t, bool);
int init (HANDLE, DWORD, mode_t, int64_t);
+ static NTSTATUS npfs_handle (HANDLE &);
static int create (fhandler_pipe *[2], unsigned, int);
static DWORD create (LPSECURITY_ATTRIBUTES, HANDLE *, HANDLE *, DWORD,
const char *, DWORD, int64_t *unique_id = NULL);
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index f8159992c..d56ea4c88 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -530,6 +530,24 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w,
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)
{
@@ -538,7 +556,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)
@@ -565,6 +583,167 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode)
return res;
}
+NTSTATUS
+fhandler_pipe::npfs_handle (HANDLE &nph)
+{
+ static NO_COPY SRWLOCK npfs_lock;
+ static NO_COPY HANDLE npfs_dirh;
+
+ NTSTATUS status = STATUS_SUCCESS;
+ OBJECT_ATTRIBUTES attr;
+ IO_STATUS_BLOCK io;
+
+ /* Lockless after first call. */
+ if (npfs_dirh)
+ {
+ nph = npfs_dirh;
+ return STATUS_SUCCESS;
+ }
+ AcquireSRWLockExclusive (&npfs_lock);
+ if (!npfs_dirh)
+ {
+ InitializeObjectAttributes (&attr, &ro_u_npfs, 0, NULL, NULL);
+ status = NtOpenFile (&npfs_dirh, FILE_READ_ATTRIBUTES | SYNCHRONIZE,
+ &attr, &io, FILE_SHARE_READ | FILE_SHARE_WRITE,
+ 0);
+ }
+ ReleaseSRWLockExclusive (&npfs_lock);
+ if (NT_SUCCESS (status))
+ nph = npfs_dirh;
+ return status;
+}
+
+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_pipe::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-%p", id);
+ if (unique_id)
+ *unique_id = ((int64_t) id << 32 | GetCurrentProcessId ());
+
+ debug_printf ("name %S, 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_buf);
+
+ 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)
{
--
2.17.0
More information about the Cygwin-patches
mailing list