[PATCH v2 0/4] MR3073: Draft: Implement FileDispositionInformationEx and FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE
This MR supercedes !1895 as a solution to bug [#50771](https://bugs.winehq.org/show_bug.cgi?id=50771). Rather than fixing the problem of Wine's inability to modify the attributes of read-only files, this patch set instead implements `FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE` which the Msys2 and Cygwin runtime libriries can use to avoid needing to modify attributes. -- v2: ntdll: Implemented FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE ntdll/tests: Added tests for FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
From: Joel Holdsworth <joel(a)airwebreathe.org.uk> Signed-off-by: Joel Holdsworth <joel(a)airwebreathe.org.uk> --- include/winternl.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/winternl.h b/include/winternl.h index 455e199f3e1..1549f5b85f4 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -1255,6 +1255,7 @@ typedef enum _FILE_INFORMATION_CLASS { FileReplaceCompletionInformation, FileHardLinkFullIdInformation, FileIdExtdBothDirectoryInformation, + FileDispositionInformationEx, FileMaximumInformation } FILE_INFORMATION_CLASS, *PFILE_INFORMATION_CLASS; @@ -1424,6 +1425,17 @@ typedef struct _FILE_DISPOSITION_INFORMATION { BOOLEAN DoDeleteFile; } FILE_DISPOSITION_INFORMATION, *PFILE_DISPOSITION_INFORMATION; +typedef struct _FILE_DISPOSITION_INFORMATION_EX { + ULONG Flags; +} FILE_DISPOSITION_INFORMATION_EX, *PFILE_DISPOSITION_INFORMATION_EX; + +#define FILE_DISPOSITION_DO_NOT_DELETE 0x00000000 +#define FILE_DISPOSITION_DELETE 0x00000001 +#define FILE_DISPOSITION_POSIX_SEMANTICS 0x00000002 +#define FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK 0x00000004 +#define FILE_DISPOSITION_ON_CLOSE 0x00000008 +#define FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE 0x00000010 + typedef struct _FILE_POSITION_INFORMATION { LARGE_INTEGER CurrentByteOffset; } FILE_POSITION_INFORMATION, *PFILE_POSITION_INFORMATION; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
From: Joel Holdsworth <joel(a)airwebreathe.org.uk> This is required by Msys2 when running gpg-agent. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54996 Signed-off-by: Joel Holdsworth <joel(a)airwebreathe.org.uk> --- dlls/ntdll/unix/file.c | 29 +++++++++++++++++++++++++++-- include/wine/server_protocol.h | 14 +++++++------- server/fd.c | 12 ++++++------ server/protocol.def | 4 ++-- server/request.h | 10 +++++----- server/trace.c | 8 ++++---- 6 files changed, 51 insertions(+), 26 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index ff1618f31fd..da6c157ba71 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4751,10 +4751,35 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, { FILE_DISPOSITION_INFORMATION *info = ptr; - SERVER_START_REQ( set_fd_disp_info ) + SERVER_START_REQ( set_fd_disp_info_ex ) { req->handle = wine_server_obj_handle( handle ); - req->unlink = info->DoDeleteFile; + req->flags = info->DoDeleteFile ? FILE_DISPOSITION_DELETE : FILE_DISPOSITION_DO_NOT_DELETE; + status = wine_server_call( req ); + } + SERVER_END_REQ; + } + else status = STATUS_INVALID_PARAMETER_3; + break; + + case FileDispositionInformationEx: + if (len >= sizeof(FILE_DISPOSITION_INFORMATION_EX)) + { + FILE_DISPOSITION_INFORMATION_EX *info = ptr; + + if (info->Flags & FILE_DISPOSITION_POSIX_SEMANTICS) + FIXME( "FILE_DISPOSITION_POSIX_SEMANTICS not supported\n" ); + if (info->Flags & FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK) + FIXME( "FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK not supported\n" ); + if (info->Flags & FILE_DISPOSITION_ON_CLOSE) + FIXME( "FILE_DISPOSITION_ON_CLOSE not supported\n" ); + if (info->Flags & FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE) + FIXME( "FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE not supported\n" ); + + SERVER_START_REQ( set_fd_disp_info_ex ) + { + req->handle = wine_server_obj_handle( handle ); + req->flags = info->Flags; status = wine_server_call( req ); } SERVER_END_REQ; diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 1af69c90b55..4c67701344a 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -5226,14 +5226,14 @@ struct set_fd_completion_mode_reply -struct set_fd_disp_info_request +struct set_fd_disp_info_ex_request { struct request_header __header; obj_handle_t handle; - int unlink; + unsigned int flags; char __pad_20[4]; }; -struct set_fd_disp_info_reply +struct set_fd_disp_info_ex_reply { struct reply_header __header; }; @@ -5819,7 +5819,7 @@ enum request REQ_set_completion_info, REQ_add_fd_completion, REQ_set_fd_completion_mode, - REQ_set_fd_disp_info, + REQ_set_fd_disp_info_ex, REQ_set_fd_name_info, REQ_set_fd_eof_info, REQ_get_window_layered_info, @@ -6106,7 +6106,7 @@ union generic_request struct set_completion_info_request set_completion_info_request; struct add_fd_completion_request add_fd_completion_request; struct set_fd_completion_mode_request set_fd_completion_mode_request; - struct set_fd_disp_info_request set_fd_disp_info_request; + struct set_fd_disp_info_ex_request set_fd_disp_info_ex_request; struct set_fd_name_info_request set_fd_name_info_request; struct set_fd_eof_info_request set_fd_eof_info_request; struct get_window_layered_info_request get_window_layered_info_request; @@ -6391,7 +6391,7 @@ union generic_reply struct set_completion_info_reply set_completion_info_reply; struct add_fd_completion_reply add_fd_completion_reply; struct set_fd_completion_mode_reply set_fd_completion_mode_reply; - struct set_fd_disp_info_reply set_fd_disp_info_reply; + struct set_fd_disp_info_ex_reply set_fd_disp_info_ex_reply; struct set_fd_name_info_reply set_fd_name_info_reply; struct set_fd_eof_info_reply set_fd_eof_info_reply; struct get_window_layered_info_reply get_window_layered_info_reply; @@ -6417,7 +6417,7 @@ union generic_reply /* ### protocol_version begin ### */ -#define SERVER_PROTOCOL_VERSION 776 +#define SERVER_PROTOCOL_VERSION 777 /* ### protocol_version end ### */ diff --git a/server/fd.c b/server/fd.c index eaebe044f37..46d32416463 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2463,7 +2463,7 @@ static int is_dir_empty( int fd ) } /* set disposition for the fd */ -static void set_fd_disposition( struct fd *fd, int unlink ) +static void set_fd_disposition_ex( struct fd *fd, unsigned int flags ) { struct stat st; @@ -2479,7 +2479,7 @@ static void set_fd_disposition( struct fd *fd, int unlink ) return; } - if (unlink) + if (flags & FILE_DISPOSITION_DELETE) { struct fd *fd_ptr; @@ -2524,7 +2524,7 @@ static void set_fd_disposition( struct fd *fd, int unlink ) } } - fd->closed->unlink = unlink ? 1 : 0; + fd->closed->unlink = (flags & FILE_DISPOSITION_DELETE) ? 1 : 0; if (fd->options & FILE_DELETE_ON_CLOSE) fd->closed->unlink = -1; } @@ -2949,13 +2949,13 @@ DECL_HANDLER(set_fd_completion_mode) } } -/* set fd disposition information */ -DECL_HANDLER(set_fd_disp_info) +/* set fd disposition extended information */ +DECL_HANDLER(set_fd_disp_info_ex) { struct fd *fd = get_handle_fd_obj( current->process, req->handle, DELETE ); if (fd) { - set_fd_disposition( fd, req->unlink ); + set_fd_disposition_ex( fd, req->flags ); release_object( fd ); } } diff --git a/server/protocol.def b/server/protocol.def index cd96858dea9..0f0732b4929 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3639,9 +3639,9 @@ struct handle_info /* set fd disposition information */ -(a)REQ(set_fd_disp_info) +(a)REQ(set_fd_disp_info_ex) obj_handle_t handle; /* handle to a file or directory */ - int unlink; /* whether to unlink file on close */ + unsigned int flags; /* what actions should be taken when deleting a file */ @END diff --git a/server/request.h b/server/request.h index 20e27ebdce1..447f93212de 100644 --- a/server/request.h +++ b/server/request.h @@ -377,7 +377,7 @@ DECL_HANDLER(query_completion); DECL_HANDLER(set_completion_info); DECL_HANDLER(add_fd_completion); DECL_HANDLER(set_fd_completion_mode); -DECL_HANDLER(set_fd_disp_info); +DECL_HANDLER(set_fd_disp_info_ex); DECL_HANDLER(set_fd_name_info); DECL_HANDLER(set_fd_eof_info); DECL_HANDLER(get_window_layered_info); @@ -663,7 +663,7 @@ static const req_handler req_handlers[REQ_NB_REQUESTS] = (req_handler)req_set_completion_info, (req_handler)req_add_fd_completion, (req_handler)req_set_fd_completion_mode, - (req_handler)req_set_fd_disp_info, + (req_handler)req_set_fd_disp_info_ex, (req_handler)req_set_fd_name_info, (req_handler)req_set_fd_eof_info, (req_handler)req_get_window_layered_info, @@ -2206,9 +2206,9 @@ C_ASSERT( sizeof(struct add_fd_completion_request) == 40 ); C_ASSERT( FIELD_OFFSET(struct set_fd_completion_mode_request, handle) == 12 ); C_ASSERT( FIELD_OFFSET(struct set_fd_completion_mode_request, flags) == 16 ); C_ASSERT( sizeof(struct set_fd_completion_mode_request) == 24 ); -C_ASSERT( FIELD_OFFSET(struct set_fd_disp_info_request, handle) == 12 ); -C_ASSERT( FIELD_OFFSET(struct set_fd_disp_info_request, unlink) == 16 ); -C_ASSERT( sizeof(struct set_fd_disp_info_request) == 24 ); +C_ASSERT( FIELD_OFFSET(struct set_fd_disp_info_ex_request, handle) == 12 ); +C_ASSERT( FIELD_OFFSET(struct set_fd_disp_info_ex_request, flags) == 16 ); +C_ASSERT( sizeof(struct set_fd_disp_info_ex_request) == 24 ); C_ASSERT( FIELD_OFFSET(struct set_fd_name_info_request, handle) == 12 ); C_ASSERT( FIELD_OFFSET(struct set_fd_name_info_request, rootdir) == 16 ); C_ASSERT( FIELD_OFFSET(struct set_fd_name_info_request, namelen) == 20 ); diff --git a/server/trace.c b/server/trace.c index 4dbc5de352f..a064a9118ea 100644 --- a/server/trace.c +++ b/server/trace.c @@ -4357,10 +4357,10 @@ static void dump_set_fd_completion_mode_request( const struct set_fd_completion_ fprintf( stderr, ", flags=%08x", req->flags ); } -static void dump_set_fd_disp_info_request( const struct set_fd_disp_info_request *req ) +static void dump_set_fd_disp_info_ex_request( const struct set_fd_disp_info_ex_request *req ) { fprintf( stderr, " handle=%04x", req->handle ); - fprintf( stderr, ", unlink=%d", req->unlink ); + fprintf( stderr, ", flags=%08x", req->flags ); } static void dump_set_fd_name_info_request( const struct set_fd_name_info_request *req ) @@ -4813,7 +4813,7 @@ static const dump_func req_dumpers[REQ_NB_REQUESTS] = { (dump_func)dump_set_completion_info_request, (dump_func)dump_add_fd_completion_request, (dump_func)dump_set_fd_completion_mode_request, - (dump_func)dump_set_fd_disp_info_request, + (dump_func)dump_set_fd_disp_info_ex_request, (dump_func)dump_set_fd_name_info_request, (dump_func)dump_set_fd_eof_info_request, (dump_func)dump_get_window_layered_info_request, @@ -5379,7 +5379,7 @@ static const char * const req_names[REQ_NB_REQUESTS] = { "set_completion_info", "add_fd_completion", "set_fd_completion_mode", - "set_fd_disp_info", + "set_fd_disp_info_ex", "set_fd_name_info", "set_fd_eof_info", "get_window_layered_info", -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
From: Joel Holdsworth <joel(a)airwebreathe.org.uk> Signed-off-by: Joel Holdsworth <joel(a)airwebreathe.org.uk> --- dlls/ntdll/tests/file.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index b96b2e5b072..96d6ab0a88a 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -2957,6 +2957,7 @@ static void test_file_disposition_information(void) NTSTATUS res; IO_STATUS_BLOCK io; FILE_DISPOSITION_INFORMATION fdi; + FILE_DISPOSITION_INFORMATION_EX fdie; FILE_STANDARD_INFORMATION fsi; BOOL fileDeleted; DWORD fdi2, size; @@ -3089,6 +3090,26 @@ static void test_file_disposition_information(void) SetFileAttributesA( buffer, FILE_ATTRIBUTE_NORMAL ); DeleteFileA( buffer ); + /* set disposition on readonly file ignoring readonly attribute */ + /* FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE is only supported in Windows 10 build 1809 and later */ + GetTempFileNameA( tmp_path, "dis", 0, buffer ); + DeleteFileA( buffer ); + handle = CreateFileA(buffer, GENERIC_WRITE | DELETE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_READONLY, 0); + ok( handle != INVALID_HANDLE_VALUE, "failed to create temp file\n" ); + fdie.Flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE; + res = pNtSetInformationFile( handle, &io, &fdie, sizeof fdie, FileDispositionInformationEx ); + todo_wine + ok( res == STATUS_INVALID_INFO_CLASS || res == STATUS_SUCCESS, + "unexpected FileDispositionInformation result (expected STATUS_SUCCES or SSTATUS_INVALID_INFO_CLASS, got %lx)\n", res ); + CloseHandle( handle ); + if ( res == STATUS_SUCCESS ) { + fileDeleted = GetFileAttributesA( buffer ) == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND; + todo_wine + ok( fileDeleted, "File should have been deleted\n" ); + } + SetFileAttributesA( buffer, FILE_ATTRIBUTE_NORMAL ); + DeleteFileA( buffer ); + /* can set disposition on file and then reset it */ GetTempFileNameA( tmp_path, "dis", 0, buffer ); handle = CreateFileA(buffer, GENERIC_WRITE | DELETE, 0, NULL, CREATE_ALWAYS, 0, 0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
From: Joel Holdsworth <joel(a)airwebreathe.org.uk> Both the Msys2 and Cygwin runtimes make use of FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE in their implementations of the unlink() system call. This enables these routines to delete a read-only file without first modifying the attributes. https://github.com/msys2/msys2-runtime/blob/msys2-3.4.3/winsup/cygwin/syscal... https://www.cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/sysca... Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50771 Signed-off-by: Joel Holdsworth <joel(a)airwebreathe.org.uk> --- dlls/ntdll/tests/file.c | 2 -- dlls/ntdll/unix/file.c | 2 -- server/fd.c | 3 ++- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index 96d6ab0a88a..34c00a94fee 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -3098,13 +3098,11 @@ static void test_file_disposition_information(void) ok( handle != INVALID_HANDLE_VALUE, "failed to create temp file\n" ); fdie.Flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE; res = pNtSetInformationFile( handle, &io, &fdie, sizeof fdie, FileDispositionInformationEx ); - todo_wine ok( res == STATUS_INVALID_INFO_CLASS || res == STATUS_SUCCESS, "unexpected FileDispositionInformation result (expected STATUS_SUCCES or SSTATUS_INVALID_INFO_CLASS, got %lx)\n", res ); CloseHandle( handle ); if ( res == STATUS_SUCCESS ) { fileDeleted = GetFileAttributesA( buffer ) == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND; - todo_wine ok( fileDeleted, "File should have been deleted\n" ); } SetFileAttributesA( buffer, FILE_ATTRIBUTE_NORMAL ); diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index da6c157ba71..cb07388a82d 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4773,8 +4773,6 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, FIXME( "FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK not supported\n" ); if (info->Flags & FILE_DISPOSITION_ON_CLOSE) FIXME( "FILE_DISPOSITION_ON_CLOSE not supported\n" ); - if (info->Flags & FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE) - FIXME( "FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE not supported\n" ); SERVER_START_REQ( set_fd_disp_info_ex ) { diff --git a/server/fd.c b/server/fd.c index 46d32416463..363a25ec925 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2499,7 +2499,8 @@ static void set_fd_disposition_ex( struct fd *fd, unsigned int flags ) } if (S_ISREG( st.st_mode )) /* can't unlink files we don't have permission to write */ { - if (!(st.st_mode & (S_IWUSR | S_IWGRP | S_IWOTH))) + if (!(flags & FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE) && + !(st.st_mode & (S_IWUSR | S_IWGRP | S_IWOTH))) { set_error( STATUS_CANNOT_DELETE ); return; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3073
@zfigura @bernhardu @julliard Any review/feedback would be welcome. Thanks -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3073#note_35810
participants (2)
-
Joel Holdsworth -
Joel Holdsworth (＠jhol)