Cygwin: fix all usages of NtQueryDirectoryObject

Due to reports on the Cygwin mailing list[1][2], it was uncovered
that a NtOpenDirectoryObject/NtQueryDirectoryObject/NtClose sequence
with NtQueryDirectoryObject iterating over the directory entries,
one entry per invocation, is not running atomically.  If new entries
are inserted into the queried directory, other entries may be moved
around and then accidentally show up twice while iterating.

Change (almost) all NtQueryDirectoryObject invocations so that it gets
a really big buffer (64K) and ideally fetches all entries at once.
This appears to work atomically.

"Almost" all, because fhandler_procsys::readdir can't be easily changed.

[1] https://cygwin.com/pipermail/cygwin/2021-July/248998.html
[2] https://cygwin.com/pipermail/cygwin/2021-August/249124.html

Fixes: e9c8cb3193 ("(format_proc_partitions): Revamp loop over existing harddisks by scanning the NT native \Device object directory and looking for Harddisk entries.")
Fixes: a998dd7055 ("Implement advisory file locking.")
Fixes: 3b7cd74bfd ("(winpids::enum_processes): Fetch Cygwin processes from listing of shared cygwin object dir in the native NT namespace.")
Fixes: 0d6f2b0117 ("syscalls.cc (sync_worker): Rewrite using native NT functions.")
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

winsup/cygwin/fhandler_proc.cc

@@ -1690,15 +1690,6 @@ format_proc_partitions (void *, char *&destbuf)
   IO_STATUS_BLOCK io;
   NTSTATUS status;
   HANDLE dirhdl;
-  tmp_pathbuf tp;
-
-  char *buf = tp.c_get ();
-  char *bufptr = buf;
-  char *ioctl_buf = tp.c_get ();
-  PWCHAR mp_buf = tp.w_get ();
-  WCHAR fpath[MAX_PATH];
-  WCHAR gpath[MAX_PATH];
-  DWORD len;
 
   /* Open \Device object directory. */
   wchar_t wpath[MAX_PATH] = L"\\Device";
@@ -1712,23 +1703,47 @@ format_proc_partitions (void *, char *&destbuf)
       return 0;
     }
 
+  tmp_pathbuf tp;
+  char *buf = tp.c_get ();
+  char *bufptr = buf;
+  char *ioctl_buf = tp.c_get ();
+  PWCHAR mp_buf = tp.w_get ();
+  PDIRECTORY_BASIC_INFORMATION dbi_buf = (PDIRECTORY_BASIC_INFORMATION)
+					 tp.w_get ();
+  WCHAR fpath[MAX_PATH];
+  WCHAR gpath[MAX_PATH];
+  DWORD len;
+
   /* Traverse \Device directory ... */
-  PDIRECTORY_BASIC_INFORMATION dbi = (PDIRECTORY_BASIC_INFORMATION)
-				     alloca (640);
   BOOLEAN restart = TRUE;
   bool got_one = false;
+  bool last_run = false;
   ULONG context = 0;
-  while (NT_SUCCESS (NtQueryDirectoryObject (dirhdl, dbi, 640, TRUE, restart,
+  while (!last_run)
-					     &context, NULL)))
+    {
+      status = NtQueryDirectoryObject (dirhdl, dbi_buf, 65536, FALSE, restart,
+				       &context, NULL);
+      if (!NT_SUCCESS (status))
+	{
+	  debug_printf ("NtQueryDirectoryObject, status %y", status);
+	  __seterrno_from_nt_status (status);
+	  break;
+	}
+      if (status != STATUS_MORE_ENTRIES)
+	last_run = true;
+      restart = FALSE;
+      for (PDIRECTORY_BASIC_INFORMATION dbi = dbi_buf;
+	   dbi->ObjectName.Length > 0;
+	   dbi++)
 	{
 	  HANDLE devhdl;
 	  PARTITION_INFORMATION_EX *pix = NULL;
 	  PARTITION_INFORMATION *pi = NULL;
 	  DWORD bytes_read;
 	  DWORD part_cnt = 0;
+	  unsigned long drive_num;
 	  unsigned long long size;
 
-      restart = FALSE;
 	  /* ... and check for a "Harddisk[0-9]*" entry. */
 	  if (dbi->ObjectName.Length < 9 * sizeof (WCHAR)
 	      || wcsncasecmp (dbi->ObjectName.Buffer, L"Harddisk", 8) != 0
@@ -1737,7 +1752,7 @@ format_proc_partitions (void *, char *&destbuf)
 	  /* Got it.  Now construct the path to the entire disk, which is
 	     "\\Device\\HarddiskX\\Partition0", and open the disk with
 	     minimum permissions. */
-      unsigned long drive_num = wcstoul (dbi->ObjectName.Buffer + 8, NULL, 10);
+	  drive_num = wcstoul (dbi->ObjectName.Buffer + 8, NULL, 10);
 	  wcscpy (wpath, dbi->ObjectName.Buffer);
 	  PWCHAR wpart = wpath + dbi->ObjectName.Length / sizeof (WCHAR);
 	  wcpcpy (wpart, L"\\Partition0");
@@ -1759,14 +1774,15 @@ format_proc_partitions (void *, char *&destbuf)
 	      got_one = true;
 	    }
 	  /* Fetch partition info for the entire disk to get its size. */
-      if (DeviceIoControl (devhdl, IOCTL_DISK_GET_PARTITION_INFO_EX, NULL, 0,
+	  if (DeviceIoControl (devhdl, IOCTL_DISK_GET_PARTITION_INFO_EX, NULL,
-			   ioctl_buf, NT_MAX_PATH, &bytes_read, NULL))
+			       0, ioctl_buf, NT_MAX_PATH, &bytes_read, NULL))
 	    {
 	      pix = (PARTITION_INFORMATION_EX *) ioctl_buf;
 	      size = pix->PartitionLength.QuadPart;
 	    }
-      else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_PARTITION_INFO, NULL, 0,
+	  else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_PARTITION_INFO, NULL,
-				ioctl_buf, NT_MAX_PATH, &bytes_read, NULL))
+				    0, ioctl_buf, NT_MAX_PATH, &bytes_read,
+				    NULL))
 	    {
 	      pi = (PARTITION_INFORMATION *) ioctl_buf;
 	      size = pi->PartitionLength.QuadPart;
@@ -1781,19 +1797,21 @@ format_proc_partitions (void *, char *&destbuf)
 	  bufptr += __small_sprintf (bufptr, "%5d %5d %9U %s\n",
 				     dev.get_major (), dev.get_minor (),
 				     size >> 10, dev.name () + 5);
-      /* Fetch drive layout info to get size of all partitions on the disk. */
+	  /* Fetch drive layout info to get size of all partitions on disk. */
-      if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX,
+	  if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, NULL, 0,
-			   NULL, 0, ioctl_buf, NT_MAX_PATH, &bytes_read, NULL))
+			       ioctl_buf, NT_MAX_PATH, &bytes_read, NULL))
 	    {
-	  PDRIVE_LAYOUT_INFORMATION_EX pdlix = (PDRIVE_LAYOUT_INFORMATION_EX)
+	      PDRIVE_LAYOUT_INFORMATION_EX pdlix =
-					       ioctl_buf;
+		  (PDRIVE_LAYOUT_INFORMATION_EX) ioctl_buf;
 	      part_cnt = pdlix->PartitionCount;
 	      pix = pdlix->PartitionEntry;
 	    }
-      else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT,
+	  else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT, NULL,
-				NULL, 0, ioctl_buf, NT_MAX_PATH, &bytes_read, NULL))
+				    0, ioctl_buf, NT_MAX_PATH, &bytes_read,
+				    NULL))
 	    {
-	  PDRIVE_LAYOUT_INFORMATION pdli = (PDRIVE_LAYOUT_INFORMATION) ioctl_buf;
+	      PDRIVE_LAYOUT_INFORMATION pdli =
+		  (PDRIVE_LAYOUT_INFORMATION) ioctl_buf;
 	      part_cnt = pdli->PartitionCount;
 	      pi = pdli->PartitionEntry;
 	    }
@@ -1819,8 +1837,8 @@ format_proc_partitions (void *, char *&destbuf)
 		    ++pi;
 		  }
 		/* A partition number of 0 denotes an extended partition or a
-	       filler entry as described in fhandler_dev_floppy::lock_partition.
+		   filler entry as described in
-	       Just skip. */
+		   fhandler_dev_floppy::lock_partition.  Just skip. */
 		if (part_num == 0)
 		  continue;
 		device dev (drive_num, part_num);
@@ -1848,6 +1866,7 @@ format_proc_partitions (void *, char *&destbuf)
 	      }
 	  NtClose (devhdl);
 	}
+    }
   NtClose (dirhdl);
 
   if (!got_one)

winsup/cygwin/flock.cc

@@ -596,24 +596,35 @@ lockf_t::from_obj_name (inode_t *node, lockf_t **head, const wchar_t *name)
 lockf_t *
 inode_t::get_all_locks_list ()
 {
-  struct fdbi
+  tmp_pathbuf tp;
-  {
-    DIRECTORY_BASIC_INFORMATION dbi;
-    WCHAR buf[2][NAME_MAX + 1];
-  } f;
   ULONG context;
   NTSTATUS status;
+  BOOLEAN restart = TRUE;
+  bool last_run = false;
   lockf_t newlock, *lock = i_all_lf;
 
-  for (BOOLEAN restart = TRUE;
+  PDIRECTORY_BASIC_INFORMATION dbi_buf = (PDIRECTORY_BASIC_INFORMATION)
-       NT_SUCCESS (status = NtQueryDirectoryObject (i_dir, &f, sizeof f, TRUE,
+					 tp.w_get ();
-						    restart, &context, NULL));
+  while (!last_run)
-       restart = FALSE)
     {
-      if (f.dbi.ObjectName.Length != LOCK_OBJ_NAME_LEN * sizeof (WCHAR))
+      status = NtQueryDirectoryObject (i_dir, dbi_buf, 65536, FALSE, restart,
+				       &context, NULL);
+      if (!NT_SUCCESS (status))
+	{
+	  debug_printf ("NtQueryDirectoryObject, status %y", status);
+	  break;
+	}
+      if (status != STATUS_MORE_ENTRIES)
+	last_run = true;
+      restart = FALSE;
+      for (PDIRECTORY_BASIC_INFORMATION dbi = dbi_buf;
+	   dbi->ObjectName.Length > 0;
+	   dbi++)
+	{
+	  if (dbi->ObjectName.Length != LOCK_OBJ_NAME_LEN * sizeof (WCHAR))
 	    continue;
-      f.dbi.ObjectName.Buffer[LOCK_OBJ_NAME_LEN] = L'\0';
+	  dbi->ObjectName.Buffer[LOCK_OBJ_NAME_LEN] = L'\0';
-      if (!newlock.from_obj_name (this, &i_all_lf, f.dbi.ObjectName.Buffer))
+	  if (!newlock.from_obj_name (this, &i_all_lf, dbi->ObjectName.Buffer))
 	    continue;
 	  if (lock - i_all_lf >= MAX_LOCKF_CNT)
 	    {
@@ -625,6 +636,7 @@ inode_t::get_all_locks_list ()
 	    lock[-1].lf_next = lock;
 	  new (lock++) lockf_t (newlock);
 	}
+    }
   /* If no lock has been found, return NULL. */
   if (lock == i_all_lf)
     return NULL;

winsup/cygwin/pinfo.cc

@@ -1526,26 +1526,38 @@ winpids::enum_processes (bool winpid)
 
   if (!winpid)
     {
+      tmp_pathbuf tp;
+      NTSTATUS status;
       HANDLE dir = get_shared_parent_dir ();
       BOOLEAN restart = TRUE;
-      ULONG context;
+      bool last_run = false;
-      struct fdbi
+      ULONG context = 0;
+      PDIRECTORY_BASIC_INFORMATION dbi_buf = (PDIRECTORY_BASIC_INFORMATION)
+					     tp.w_get ();
+      while (!last_run)
 	{
-	  DIRECTORY_BASIC_INFORMATION dbi;
+	  status = NtQueryDirectoryObject (dir, dbi_buf, 65536, FALSE, restart,
-	  WCHAR buf[2][NAME_MAX + 1];
+					   &context, NULL);
-	} f;
+	  if (!NT_SUCCESS (status))
-      while (NT_SUCCESS (NtQueryDirectoryObject (dir, &f, sizeof f, TRUE,
-						 restart, &context, NULL)))
 	    {
+	      debug_printf ("NtQueryDirectoryObject, status %y", status);
+	      break;
+	    }
+	  if (status != STATUS_MORE_ENTRIES)
+	    last_run = true;
 	  restart = FALSE;
-	  f.dbi.ObjectName.Buffer[f.dbi.ObjectName.Length / sizeof (WCHAR)] = L'\0';
+	  for (PDIRECTORY_BASIC_INFORMATION dbi = dbi_buf;
-	  if (wcsncmp (f.dbi.ObjectName.Buffer, L"winpid.", 7) == 0)
+	       dbi->ObjectName.Length > 0;
+	       dbi++)
 	    {
-	      DWORD pid = wcstoul (f.dbi.ObjectName.Buffer + 7, NULL, 10);
+	      if (wcsncmp (dbi->ObjectName.Buffer, L"winpid.", 7) == 0)
+		{
+		  DWORD pid = wcstoul (dbi->ObjectName.Buffer + 7, NULL, 10);
 		  add (nelem, false, pid);
 		}
 	    }
 	}
+    }
   else
     {
       static DWORD szprocs;

winsup/cygwin/release/3.3.0

@@ -62,3 +62,8 @@ Bug Fixes
 - Fix getaddrinfo(3) to return valid ai_socktype and ai_protocol values
   if the underlying GetAddrInfoW screwes up.
   Addresses: https://cygwin.com/pipermail/cygwin/2021-July/248985.html
+
+- Fix duplicate /proc/partitions entries and (presumably) duplicate PIDs
+  in ps(1) output.
+  Addresses: https://cygwin.com/pipermail/cygwin/2021-July/248998.html
+             https://cygwin.com/pipermail/cygwin/2021-August/249124.html

winsup/cygwin/syscalls.cc

@@ -2050,19 +2050,35 @@ sync ()
       return;
     }
   /* Traverse \Device directory ... */
-  PDIRECTORY_BASIC_INFORMATION dbi = (PDIRECTORY_BASIC_INFORMATION)
+  tmp_pathbuf tp;
-				     alloca (640);
+  PDIRECTORY_BASIC_INFORMATION dbi_buf = (PDIRECTORY_BASIC_INFORMATION)
+					 tp.w_get ();
   BOOLEAN restart = TRUE;
+  bool last_run = false;
   ULONG context = 0;
-  while (NT_SUCCESS (NtQueryDirectoryObject (devhdl, dbi, 640, TRUE, restart,
+  while (!last_run)
-					     &context, NULL)))
     {
+      status = NtQueryDirectoryObject (devhdl, dbi_buf, 65536, FALSE, restart,
+				       &context, NULL);
+      if (!NT_SUCCESS (status))
+	{
+	  debug_printf ("NtQueryDirectoryObject, status %y", status);
+	  break;
+	}
+      if (status != STATUS_MORE_ENTRIES)
+	last_run = true;
       restart = FALSE;
+      for (PDIRECTORY_BASIC_INFORMATION dbi = dbi_buf;
+	   dbi->ObjectName.Length > 0;
+	   dbi++)
+	{
 	  /* ... and call sync_worker for each HarddiskVolumeX entry. */
 	  if (dbi->ObjectName.Length >= 15 * sizeof (WCHAR)
 	      && !wcsncasecmp (dbi->ObjectName.Buffer, L"HarddiskVolume", 14)
 	      && iswdigit (dbi->ObjectName.Buffer[14]))
-	sync_worker (devhdl, dbi->ObjectName.Length, dbi->ObjectName.Buffer);
+	    sync_worker (devhdl, dbi->ObjectName.Length,
+			 dbi->ObjectName.Buffer);
+	}
     }
   NtClose (devhdl);
 }