[PATCH 0/2] Fix RAM disk crash

[PATCH 0/2] Fix RAM disk crash

[Les De Ridder]

Some executables cause a bugcheck when running Cygwin off a native RAM
disk, which AFAICT is caused by a buggy (Microsoft) driver. This issue
will most likely only occur on WinPE and similar environments (tested on
10.0.20348.1).

See <https://github.com/msys2/msys2-runtime/issues/160> for some
context.

I provide my patches under both the 2-clause BSD license and the CC0
license.

Les De Ridder (2):
  Detect RAM disks as a separate filesystem type
  Use init_reopen_attr in mmap

 winsup/cygwin/local_includes/mount.h |  2 ++
 winsup/cygwin/local_includes/path.h  |  1 +
 winsup/cygwin/mm/mmap.cc             |  5 ++---
 winsup/cygwin/mount.cc               | 12 ++++++++++++
 4 files changed, 17 insertions(+), 3 deletions(-)

-- 
2.41.0

[PATCH 1/2] Detect RAM disks as a separate filesystem type

[Les De Ridder]

Native RAM disks, e.g. as are used in WinPE environments, have other
characteristics than regular filesystems such as NTFS. For instance,
re-opening files with NtOpenFile is buggy.

This commit checks whether a volume is a RAM disk through the NT object
the drive letter link refers to. This seems to be the only reliable
method of checking whether a volume is a native RAM disk.

Signed-off-by: Les De Ridder <les@lesderid.net>
---
 winsup/cygwin/local_includes/mount.h |  2 ++
 winsup/cygwin/local_includes/path.h  |  1 +
 winsup/cygwin/mount.cc               | 12 ++++++++++++
 3 files changed, 15 insertions(+)

diff --git a/winsup/cygwin/local_includes/mount.h b/winsup/cygwin/local_includes/mount.h
index 5bb84b976..e8a71a994 100644
--- a/winsup/cygwin/local_includes/mount.h
+++ b/winsup/cygwin/local_includes/mount.h
@@ -47,6 +47,7 @@ enum fs_info_type
   ncfsd,
   afs,
   prlfs,
+  ramdisk,
   /* Always last. */
   max_fs_type
 };
@@ -117,6 +118,7 @@ class fs_info
   IMPLEMENT_FS_FLAG (ncfsd)
   IMPLEMENT_FS_FLAG (afs)
   IMPLEMENT_FS_FLAG (prlfs)
+  IMPLEMENT_FS_FLAG (ramdisk)
   fs_info_type what_fs () const { return status.fs_type; }
   bool got_fs () const { return status.fs_type != none; }
 
diff --git a/winsup/cygwin/local_includes/path.h b/winsup/cygwin/local_includes/path.h
index 74f831e53..2e34f0e18 100644
--- a/winsup/cygwin/local_includes/path.h
+++ b/winsup/cygwin/local_includes/path.h
@@ -387,6 +387,7 @@ class path_conv
   bool fs_is_ncfsd () const {return fs.is_ncfsd ();}
   bool fs_is_afs () const {return fs.is_afs ();}
   bool fs_is_prlfs () const {return fs.is_prlfs ();}
+  bool fs_is_ramdisk () const {return fs.is_ramdisk ();}
   fs_info_type fs_type () const {return fs.what_fs ();}
   ULONG fs_serial_number () const {return fs.serial_number ();}
   inline const char *set_path (const char *p)
diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
index 36ab042a7..1950dadb0 100644
--- a/winsup/cygwin/mount.cc
+++ b/winsup/cygwin/mount.cc
@@ -292,6 +292,17 @@ fs_info::update (PUNICODE_STRING upath, HANDLE in_vol)
   if (!NT_SUCCESS (status))
     ffdi.DeviceType = ffdi.Characteristics = 0;
 
+  if (upath->Buffer[5] == L':' && upath->Buffer[6] == L'\\')
+   {
+     WCHAR dos[3] = {upath->Buffer[4], upath->Buffer[5], L'\0'};
+     WCHAR dev[MAX_PATH];
+     if (QueryDosDeviceW (dos, dev, MAX_PATH))
+       {
+          is_ramdisk (wcsncmp (dev, L"\\Device\\Ramdisk", 15));
+          has_buggy_reopen (is_ramdisk ());
+       }
+   }
+
   if ((ffdi.Characteristics & FILE_REMOTE_DEVICE)
       || (!ffdi.DeviceType
 	  && RtlEqualUnicodePathPrefix (attr.ObjectName, &ro_u_uncp, TRUE)))
@@ -1612,6 +1623,7 @@ fs_names_t fs_names[] = {
     { "ncfsd", false },
     { "afs", false },
     { "prlfs", false },
+    { "ramdisk", false },
     { NULL, false }
 };
 
-- 
2.41.0

Re: [PATCH 1/2] Detect RAM disks as a separate filesystem type

[Corinna Vinschen]

Hi Les,

your 2nd patch looks good to me, but this patch is a bit questionable.

On Aug  7 03:13, Les De Ridder wrote:
> diff --git a/winsup/cygwin/mount.cc b/winsup/cygwin/mount.cc
> index 36ab042a7..1950dadb0 100644
> --- a/winsup/cygwin/mount.cc
> +++ b/winsup/cygwin/mount.cc
> @@ -292,6 +292,17 @@ fs_info::update (PUNICODE_STRING upath, HANDLE in_vol)
>    if (!NT_SUCCESS (status))
>      ffdi.DeviceType = ffdi.Characteristics = 0;
>  
> +  if (upath->Buffer[5] == L':' && upath->Buffer[6] == L'\\')

If that's testing for a local drive path, it's probably wrong.  The
NtQueryVolumeInformationFile(FileFsDeviceInformation) call returns the
FILE_REMOTE_DEVICE flag and sets is_remote_drive() in line 298.  You
should use that flag.

This also means your code is called much too early in fs_info::update.
The preceeding code for file systems not providing valid serial numbers
is an exception from the rule because we need the serial number for
caching.  Generically checking for another local filesystem should be
performed after the  "if (is_remote_drive ())" expression, so starting
somewhere below line 495.

> +   {
> +     WCHAR dos[3] = {upath->Buffer[4], upath->Buffer[5], L'\0'};
> +     WCHAR dev[MAX_PATH];
> +     if (QueryDosDeviceW (dos, dev, MAX_PATH))
> +       {
> +          is_ramdisk (wcsncmp (dev, L"\\Device\\Ramdisk", 15));
> +          has_buggy_reopen (is_ramdisk ());
> +       }
> +   }
> +

This gives me headaches.  Did you check *all* the information returned
by the various NtQueryVolumeInformationFile calls?  I. e., what is
returned by all these calls?  What is the FS name set to?  Which flags
are set in FileSystemAttributes?  What is DeviceType and Characterisitics
set to?

We should really check all info already available from the
NtQueryVolumeInformationFile calls first, and please paste here the
information you get from these calls.

Also, even if all else fails, rather than calling QueryDosDeviceW we
should use NtQueryVolumeInformationFile(FileFsDriverPathInformation)
instead.


Thanks,
Corinna

[PATCH 2/2] Use init_reopen_attr in mmap

[Les De Ridder]

Calling mmap on a file stored on a volume with buggy file re-opening
currently bugchecks. This commit solves this by using the
init_reopen_attr helper function.

Signed-off-by: Les De Ridder <les@lesderid.net>
---
 winsup/cygwin/mm/mmap.cc | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/mm/mmap.cc b/winsup/cygwin/mm/mmap.cc
index 332c015a7..23bbc3a98 100644
--- a/winsup/cygwin/mm/mmap.cc
+++ b/winsup/cygwin/mm/mmap.cc
@@ -956,11 +956,10 @@ mmap (void *addr, size_t len, int prot, int flags, int fd, off_t off)
       HANDLE h;
       IO_STATUS_BLOCK io;
 
-      InitializeObjectAttributes (&attr, &ro_u_empty, fh->pc.objcaseinsensitive (),
-				  fh->get_handle (), NULL);
       status = NtOpenFile (&h,
 			   fh->get_access () | GENERIC_EXECUTE | SYNCHRONIZE,
-			   &attr, &io, FILE_SHARE_VALID_FLAGS,
+			   fh->pc.init_reopen_attr (attr, h), &io,
+			   FILE_SHARE_VALID_FLAGS,
 			   FILE_SYNCHRONOUS_IO_NONALERT
 			   | FILE_OPEN_FOR_BACKUP_INTENT);
       if (NT_SUCCESS (status))
-- 
2.41.0