[PATCH v3 3/3] server: Allow to open read-only files with FILE_WRITE_ATTRIBUTES

[Joel Holdsworth]

From: Joel Holdsworth

The Msys2 port of the pacman package manager creates a read-only lock file during package
installation. When it is time to delete this files, pacman calls the Msys2 implementation of
unlink(2). This function is implemented in msys2-runtime:

https://github.com/msys2/msys2-runtime/blob/msys2-3.4.3/winsup/cygwin/syscal...

A similar implementation exists in Cygwin:

https://www.cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/sysca...

The implementation works by first opening the file using NtOpenFile with FILE_WRITE_ATTRIBUTES
permissions only, it then clears the FILE_ATTRIBUTE_READONLY flag using NtSetAttributesFile, then
repoens the file with DELETE permissions.

Previously, the Wine implementation of NtOpenFile would fail to open read-only files for
FILE_WRITE_ATTRIBUTES access in wine-server, because it would attempt to open the file with O_RDWR
or O_WRONLY access.

This patch solves this issue by only opening the file for writing if O_CREAT or O_TRUNC is
requested, or if the caller has requested other write-access flags in addition. Otherwise, read-only
access is sufficient to allow NtSetAttributesFile to operate using fchmod(2).

This patch was inspired by the following wine-staging patches:

server-File_Permissions/0007-server-FILE_WRITE_ATTRIBUTES-should-succeed-for-read.patch
server-File_Permissions/0002-server-Allow-to-open-files-without-any-permission-bi.patch

These staging patches address the case where a file has neither read or write access flags, and
allows the file to be opened by temporarily modifying the flags to grant access.

This patch takes a simpler approach of assuming that read-only files will have the read access flag
already granted. In this case a read-only file can be opened with O_RDONLY access, providing an fd
that is usable with fchmod(2) etc.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50771
Co-authored-by: Sebastian Lackner 
Signed-off-by: Joel Holdsworth 
---
 dlls/ntdll/tests/file.c | 36 +++++++++++++++---------------------
 server/fd.c             |  7 ++++++-
 2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c
index 57dee54076a..fb02fd4a527 100644
--- a/dlls/ntdll/tests/file.c
+++ b/dlls/ntdll/tests/file.c
@@ -5318,33 +5318,27 @@ static void test_file_readonly_access(void)

/* NtOpenFile FILE_WRITE_ATTRIBUTES */
     status = pNtOpenFile(&handle, FILE_WRITE_ATTRIBUTES,  &attr, &io, default_sharing, FILE_NON_DIRECTORY_FILE);
-    todo_wine ok(status == STATUS_SUCCESS, "got %#lx\n", status);
-
-    if (status == STATUS_SUCCESS) {
-        memset(&fbi, 0, sizeof(fbi));
-        fbi.FileAttributes = FILE_ATTRIBUTE_NORMAL;
-        U(io).Status = 0xdeadbeef;
-        status = pNtSetInformationFile(handle, &io, &fbi, sizeof fbi, FileBasicInformation);
-        todo_wine ok(status == STATUS_SUCCESS, "can't set system attribute, NtSetInformationFile returned %#lx\n",
-                     status);
-        todo_wine ok(U(io).Status == STATUS_SUCCESS, "can't set system attribute, io.Status is %lx\n",
-                     U(io).Status);
-
-        fbi.FileAttributes = FILE_ATTRIBUTE_READONLY;
-        U(io).Status = 0xdeadbeef;
-        status = pNtSetInformationFile(handle, &io, &fbi, sizeof fbi, FileBasicInformation);
-        todo_wine ok(status == STATUS_SUCCESS, "can't set system attribute, NtSetInformationFile returned %#lx\n",
-                     status);
-        todo_wine ok(U(io).Status == STATUS_SUCCESS, "can't set system attribute, io.Status is %lx\n",
-                     U(io).Status);
-    }
+    ok(status == STATUS_SUCCESS, "got %#lx\n", status);
+
+    memset(&fbi, 0, sizeof(fbi));
+    fbi.FileAttributes = FILE_ATTRIBUTE_NORMAL;
+    U(io).Status = 0xdeadbeef;
+    status = pNtSetInformationFile(handle, &io, &fbi, sizeof fbi, FileBasicInformation);
+    ok(status == STATUS_SUCCESS, "can't set system attribute, NtSetInformationFile returned %#lx\n", status);
+    ok(U(io).Status == STATUS_SUCCESS, "can't set system attribute, io.Status is %lx\n", U(io).Status);
+
+    fbi.FileAttributes = FILE_ATTRIBUTE_READONLY;
+    U(io).Status = 0xdeadbeef;
+    status = pNtSetInformationFile(handle, &io, &fbi, sizeof fbi, FileBasicInformation);
+    ok(status == STATUS_SUCCESS, "can't set system attribute, NtSetInformationFile returned %#lx\n", status);
+    ok(U(io).Status == STATUS_SUCCESS, "can't set system attribute, io.Status is %lx\n", U(io).Status);

CloseHandle(handle);

/* NtOpenFile FILE_READ_ATTRIBUTES and FILE_WRITE_ATTRIBUTES */
     status = pNtOpenFile(&handle, FILE_READ_ATTRIBUTES|FILE_WRITE_ATTRIBUTES,  &attr, &io,
                          default_sharing, FILE_NON_DIRECTORY_FILE);
-    todo_wine ok(status == STATUS_SUCCESS, "got %#lx\n", status);
+    ok(status == STATUS_SUCCESS, "got %#lx\n", status);
     CloseHandle(handle);

/* NtCreateFile FILE_GENERIC_WRITE */
diff --git a/server/fd.c b/server/fd.c
index 839a1dec914..9f902c871be 100644
--- a/server/fd.c
+++ b/server/fd.c
@@ -1941,7 +1941,12 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam
         flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
     }

-    if ((access & (FILE_WRITE_DATA|FILE_APPEND_DATA|FILE_WRITE_ATTRIBUTES|FILE_WRITE_EA)) &&
+    /* If FILE_WRITE_ATTRIBUTES has been requested, write access is only required if the file
+     * will be created or modified. Otherwise, read-only access is sufficient, which is all
+     * that will be available if the file has the FILE_ATTRIBUTE_READONLY attribute.
+     */
+    if (((access & (FILE_WRITE_DATA|FILE_APPEND_DATA|FILE_WRITE_EA)) ||
+        ((access & FILE_WRITE_ATTRIBUTES) && (flags & (O_CREAT | O_TRUNC)))) &&
         !(options & FILE_DIRECTORY_FILE))
     {
         if (access & (FILE_READ_DATA|FILE_READ_ATTRIBUTES|FILE_READ_EA)) rw_mode = O_RDWR;