[PATCH] Cygwin: open: Do not set tentative fhandler to fdtab (A)

Tentative assignment of fhandler to fdtab introduced by the commit
524d75ff7398 ("Cygwin: open: Unlock fdtab before open_with_arch()")
causes the undesired behaviour. The commit intended that fhandler
was just a marker for reservation of fd. However, another cygwin
call may assume that the fd is valid and in use, and may operate on
it.

This patch introduces a flag for fdtab that means the fd is reserved
and cannot be assigned for another open(), etc.

Fixes: 524d75ff7398 ("Cygwin: open: Unlock fdtab before open_with_arch()")
Suggested-by: Johannes Schindelin <[email protected]>
Signed-off-by: Takashi Yano <[email protected]>
Reviewed-by:
---
 winsup/cygwin/dtable.cc               | 17 ++++++++++++++++-
 winsup/cygwin/local_includes/dtable.h |  2 ++
 winsup/cygwin/syscalls.cc             |  7 ++++---
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index e4d1cdf8f..78eb7e3d9 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -73,6 +73,7 @@ dtable::extend (size_t howmuch, size_t min)
 {
   size_t new_size = size + howmuch;
   fhandler_base **newfds;
+  bool *new_reserved;
 
   if (new_size <= OPEN_MAX)
     /* ok */;
@@ -93,14 +94,28 @@ dtable::extend (size_t howmuch, size_t min)
       set_errno (ENOMEM);
       return 0;
     }
+  if (!(new_reserved =
+	(bool *) ccalloc (HEAP_ARGV, new_size, sizeof new_reserved[0])))
+    {
+      debug_printf ("calloc failed");
+      set_errno (ENOMEM);
+      cfree (newfds);
+      return 0;
+    }
   if (fds)
     {
       memcpy (newfds, fds, size * sizeof (fds[0]));
       cfree (fds);
     }
+  if (reserved)
+    {
+      memcpy (new_reserved, reserved, size * sizeof (reserved[0]));
+      cfree (reserved);
+    }
 
   size = new_size;
   fds = newfds;
+  reserved = new_reserved;
   debug_printf ("size %ld, fds %p", size, fds);
   return 1;
 }
@@ -233,7 +248,7 @@ dtable::find_unused_handle (size_t start)
   do
     {
       for (size_t i = start; i < size; i++)
-	if (fds[i] == NULL)
+	if (fds[i] == NULL && !reserved[i])
 	  {
 	    res = (int) i;
 	    goto out;
diff --git a/winsup/cygwin/local_includes/dtable.h b/winsup/cygwin/local_includes/dtable.h
index 7803fae1b..e3ba48228 100644
--- a/winsup/cygwin/local_includes/dtable.h
+++ b/winsup/cygwin/local_includes/dtable.h
@@ -21,6 +21,7 @@ class dtable
 {
   fhandler_base **fds;
   fhandler_base **archetypes;
+  bool *reserved;
   unsigned narchetypes;
   unsigned farchetype;
   static const int initial_archetype_size = 8;
@@ -76,6 +77,7 @@ public:
   void fixup_before_fork (DWORD win_proc_id);
   void lock () {lock_process::locker.acquire ();}
   void unlock () {lock_process::locker.release ();}
+  bool &reserve (int fd) { return reserved[fd]; }
 };
 
 fhandler_base *build_fh_dev (const device&, const char * = NULL);
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 5465d6c09..1d5e258dd 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -1558,8 +1558,8 @@ open (const char *unix_path, int flags, ...)
 	  cygheap->fdtab.unlock ();
 	  __leave;		/* errno already set */
 	}
-      cygheap->fdtab[fd] = fh; /* tentative setting to mark as used */
-      cygheap->fdtab.unlock();
+      cygheap->fdtab.reserve (fd) = true;
+      cygheap->fdtab.unlock ();
 
       if (fh->dev () == FH_PROCESSFD && fh->pc.follow_fd_symlink ())
 	{
@@ -1590,6 +1590,7 @@ open (const char *unix_path, int flags, ...)
       cygheap->fdtab.lock ();
       cygheap->fdtab[fd] = fh;
       fh->inc_refcnt ();
+      cygheap->fdtab.reserve (fd) = false;
       cygheap->fdtab.unlock ();
 
       if (fd <= 2)
@@ -1601,7 +1602,7 @@ open (const char *unix_path, int flags, ...)
     if (res < 0 && fd >= 0)
       {
 	cygheap->fdtab.lock ();
-	cygheap->fdtab[fd] = NULL; /* Mark as unused */
+	cygheap->fdtab.reserve (fd) = false;
 	cygheap->fdtab.unlock ();
       }
   if (res < 0 && fh)
-- 
2.51.0