[newlib-cygwin/main] Cygwin: open: always fix up cached DOS file attributes after NtCreateFile

[Corinna Vinschen]

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=2d81f6ebe3dca62d594ed650a885449d66b4a7b8

commit 2d81f6ebe3dca62d594ed650a885449d66b4a7b8
Author:     Corinna Vinschen <corinna@vinschen.de>
AuthorDate: Tue Apr 8 15:52:38 2025 +0200
Commit:     Corinna Vinschen <corinna@vinschen.de>
CommitDate: Tue Apr 8 16:05:56 2025 +0200

    Cygwin: open: always fix up cached DOS file attributes after NtCreateFile
    
    If two threads try to create a file in parallel, only one of them
    actually creates the file.  So even if both threads first found the
    file non-existant, fhandler_base::open will get io.Information ==
    FILE_CREATED from NtCreateFile in one thread only, and the thread
    just opening the just created file will continue with broken DOS file
    attributes.
    
    Make sure to fix up DOS attributes all the time, not only when the file
    got created by this thread, but also when it has been just opened.
    
    Fixes: 41de4b6fd735 ("Cygwin: fix up cached DOS file attributes after file creation")
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/fhandler/base.cc | 35 ++++++++++++++++++++++++++---------
 winsup/cygwin/release/3.6.1    |  4 ++++
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/winsup/cygwin/fhandler/base.cc b/winsup/cygwin/fhandler/base.cc
index a5d15c706b2c..36d814bf3543 100644
--- a/winsup/cygwin/fhandler/base.cc
+++ b/winsup/cygwin/fhandler/base.cc
@@ -526,8 +526,9 @@ fhandler_base::open (int flags, mode_t mode)
   ULONG file_attributes = 0;
   ULONG shared = (get_major () == DEV_TAPE_MAJOR ? 0 : FILE_SHARE_VALID_FLAGS);
   ULONG create_disposition;
+  FILE_BASIC_INFORMATION fbi;
   OBJECT_ATTRIBUTES attr;
-  IO_STATUS_BLOCK io;
+  IO_STATUS_BLOCK io, io_bi;
   NTSTATUS status;
   PFILE_FULL_EA_INFORMATION p = NULL;
   ULONG plen = 0;
@@ -719,16 +720,32 @@ fhandler_base::open (int flags, mode_t mode)
 	goto done;
    }
 
-  if (io.Information == FILE_CREATED)
-    {
-      /* Correct file attributes are needed for later use in, e.g. fchmod. */
-      FILE_BASIC_INFORMATION fbi;
+  /* Fix up file attributes, they are desperately needed later.
+
+     Originally we only did that in the FILE_CREATED case below, but that's
+     insufficient:
 
-      if (!NT_SUCCESS (NtQueryInformationFile (fh, &io, &fbi, sizeof fbi,
-					       FileBasicInformation)))
-	fbi.FileAttributes = file_attributes | FILE_ATTRIBUTE_ARCHIVE;
-      pc.file_attributes (fbi.FileAttributes);
+     If two threads try to create the same file at the same time, it's
+     possible that path_conv::check returns the file as non-existant, i. e.,
+     pc.file_attributes () returns INVALID_FILE_ATTRIBUTES, 0xffffffff.
+     However, one of the NtCreateFile will beat the other, so only one of
+     them returns with FILE_CREATED.
 
+     The other fhandler_base::open() will instead run into the O_TRUNC
+     conditional (further below), blindly check for the SPARSE attribute
+     and remove that bit.  The result is that the attributes will be
+     0xfffffdff, i.e., everything but SPARSE.  Most annoying is that
+     pc.isdir() will return TRUE.  Hilarity ensues.
+
+     Note that we use a different IO_STATUS_BLOCK, so as not to overwrite
+     io.Information... */
+  if (!NT_SUCCESS (NtQueryInformationFile (fh, &io_bi, &fbi, sizeof fbi,
+					   FileBasicInformation)))
+    fbi.FileAttributes = file_attributes | FILE_ATTRIBUTE_ARCHIVE;
+  pc.file_attributes (fbi.FileAttributes);
+
+  if (io.Information == FILE_CREATED)
+    {
       /* Always create files using a NULL SD.  Create correct permission bits
 	 afterwards, maintaining the owner and group information just like
 	 chmod.  This is done for two reasons.
diff --git a/winsup/cygwin/release/3.6.1 b/winsup/cygwin/release/3.6.1
index e1bdd17271a2..15eaf488421c 100644
--- a/winsup/cygwin/release/3.6.1
+++ b/winsup/cygwin/release/3.6.1
@@ -41,3 +41,7 @@ Fixes:
   Addresses: https://cygwin.com/pipermail/cygwin/2025-April/257862.html
 
 - Fix tcsetattr() for console which has been broken sinse cygwin 3.5.5.
+
+- Fix up cached DOS attributes when trying to create the same file in
+  two (or more) threads/processes concurrently.
+  Addresses: https://cygwin.com/pipermail/cygwin/2025-April/257871.html