From: Joel Holdsworth joel@airwebreathe.org.uk
The Msys2 port of the pacman package manager creates a read-only lock file during package installation. When it is time to delete this file, 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.
Wine uses POSIX file permissions as one its possible methods of storing the READONLY DOS attribute in addition to storing attributes in Samba-formatted user extentded attributes.
Files that are opened with FILE_WRITE_ATTRIBUTES must be opened with O_WRONLY or O_RDWR, because write access is required to modify the extended attribute. However, this will fail if the POSIX file permissions are set to read-only.
This patch solves the problem by temporarily modifying the file access permissions to enable it to be opened before restoring the permissions to their previous values.
There are risks involved in doing this. Modifying the file permissions is a 4-step process:
1. stat - read the existing access flags 2. chmod - modify to allow write permission. 3. open - attempt to open the file. 4. fchmod/chmod - restore the access flags using the fd if possible.
The process involves resolving the file path at least 3 times, which is somewhat vulnerable to TOCTOU race conditions. However, the timing will be very brief, and this process occurs in server meaning that other Wine threads will be excluded. Therefore, the impact is judged to be low.
This patch was derived 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
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50771 Co-authored-by: Sebastian Lackner sebastian@fds-team.de Signed-off-by: Joel Holdsworth joel@airwebreathe.org.uk --- dlls/ntdll/tests/file.c | 36 +++++++++++++++--------------------- server/fd.c | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 23 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..8966c8e0648 100644 --- a/server/fd.c +++ b/server/fd.c @@ -1899,6 +1899,7 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam int root_fd = -1; int rw_mode; char *path; + int restore_chmod = 0;
if (((options & FILE_DELETE_ON_CLOSE) && !(access & DELETE)) || ((options & FILE_DIRECTORY_FILE) && (flags & O_TRUNC))) @@ -1958,6 +1959,29 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam (flags & O_CREAT)) fd->unix_fd = open( name, O_RDONLY | (flags & ~(O_TRUNC | O_CREAT | O_EXCL)), *mode ); } + else if (errno == EACCES) + { + /* try to change permissions temporarily to open a file descriptor */ + if ((access & FILE_WRITE_ATTRIBUTES) && + !(access & (FILE_WRITE_DATA | FILE_APPEND_DATA | FILE_WRITE_EA | + FILE_READ_DATA | FILE_READ_EA | DELETE)) && + !stat( name, &st ) && st.st_uid == getuid() && + !chmod( name, st.st_mode | S_IRUSR )) + { + fd->unix_fd = open( name, O_RDONLY | (flags & ~(O_TRUNC | O_CREAT | O_EXCL)), *mode ); + if (fd->unix_fd == -1) + chmod( name, st.st_mode ); + else + fchmod( fd->unix_fd, st.st_mode ); + *mode = st.st_mode; + restore_chmod = 1; + } + else + { + set_error( STATUS_ACCESS_DENIED ); + goto error; + } + }
if (fd->unix_fd == -1) { @@ -1966,6 +1990,8 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam set_error( STATUS_OBJECT_NAME_INVALID ); else file_set_error(); + + if (restore_chmod) chmod( name, *mode ); goto error; } } @@ -1981,8 +2007,14 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam closed_fd->unix_fd = fd->unix_fd; closed_fd->unlink = 0; closed_fd->unix_name = fd->unix_name; - fstat( fd->unix_fd, &st ); - *mode = st.st_mode; + + if (restore_chmod) + chmod( name, *mode ); + else + { + fstat( fd->unix_fd, &st ); + *mode = st.st_mode; + }
/* only bother with an inode for normal files and directories */ if (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode))