[PATCH draft v2 4/6] Cygwin: fhandler_pipe: fix permission problem

[Ken Brown <kbrown at cornell dot edu>]

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 fhandler_pipe::create (fhandler_pipe *[2],
unsigned, int), which is the version of fhandler_pipe::create used by
the pipe and pipe2 system calls.

For convenience, also add a static member function
fhandler_pipe::npfs_handle similar to those used by fhandler_fifo and
fhandler_socket_unix.
---
 winsup/cygwin/fhandler.h       |   1 +
 winsup/cygwin/fhandler_pipe.cc | 189 ++++++++++++++++++++++++++++++++-
 2 files changed, 188 insertions(+), 2 deletions(-)

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 7640fae36..27fdc8e76 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -35,7 +35,12 @@ fhandler_pipe::fhandler_pipe ()
   need_fork_fixup (true);
 }
 
-/* This also sets the pipe's read mode to byte_stream unconditionally. */
+/* The following function is intended for fhandler_pipe objects
+   created by the second version of fhandler_pipe::create below.  See
+   the comment preceding the latter.
+
+   In addition to setting the blocking mode of the pipe handle, it
+   also sets the pipe's read mode to byte_stream unconditionally. */
 void
 fhandler_pipe::set_pipe_non_blocking (bool nonblocking)
 {
@@ -566,6 +571,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)
 {
@@ -574,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)
@@ -601,6 +624,168 @@ 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-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)
 {
-- 
2.21.0