[PATCH 0/2] MR2433: ntdll: Open application manifest files with FILE_SHARE_DELETE.
Today, RtlCreateActivationContext (CreateActCtxW) opens the source manifest file via NtOpenFile without the FILE_SHARE_DELETE sharing mode. This causes CreateActCtxW to fail if the source manifest file was created with the FILE_DELETE_ON_CLOSE flag. FILE_DELETE_ON_CLOSE is often used for temporary files that should be automatically deleted after use, even if the creator process crashes. Fix this by specifying FILE_SHARE_DELETE for sharing mode when opening the source manifest or module file. This allows the source manifest or module file to be marked as deleted while it is open. Note that concurrent deletion is not an issue for the following reasons: - The ability to read from an open file handle is unaffected by deletion of the corresponding file's name. - RtlCreateActivationContext does not open the source manifest or module file by the given filename (lpSource) more than once. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2433
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/kernel32/tests/actctx.c | 67 ++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index bec04b7f066..9dbf77bc761 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -617,6 +617,50 @@ static BOOL create_wide_manifest(const char *filename, const char *manifest, BOO return ret; } +static HANDLE create_temp_manifest_file(const char *manifest, WCHAR *pathname) +{ + WCHAR tmp_path[MAX_PATH]; + DWORD size, tmp_path_len; + HANDLE file, file_w; + UINT unique; + + tmp_path_len = GetTempPathW(ARRAY_SIZE(tmp_path), tmp_path); + ok(tmp_path_len != 0, "GetTempPathW returned error %lu\n", GetLastError()); + ok(tmp_path_len < ARRAY_SIZE(tmp_path), "GetTempPathW return value %lu should be less than %Iu\n", + tmp_path_len, ARRAY_SIZE(tmp_path)); + + memset(pathname, 0, MAX_PATH * sizeof(WCHAR)); + unique = GetTempFileNameW(tmp_path, L"tst", 0, pathname); + ok(unique != 0, "GetTempFileNameW returned error %lu\n", GetLastError()); + + /* Open file handle that will be deleted on close or process termination */ + file = CreateFileW(pathname, + GENERIC_READ | DELETE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, + CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_DELETE_ON_CLOSE, + NULL); + ok(file != INVALID_HANDLE_VALUE, "CreateFile returned error %lu\n", GetLastError()); + + /* Re-open file with write access */ + file_w = CreateFileW(pathname, + GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, + TRUNCATE_EXISTING, + FILE_ATTRIBUTE_NORMAL, + NULL); + ok(file_w != INVALID_HANDLE_VALUE, "CreateFile returned error %lu\n", GetLastError()); + + WriteFile(file_w, manifest, strlen(manifest), &size, NULL); + + /* Close file handle that was open for write to avoid sharing violation */ + CloseHandle(file_w); + + return file; +} + typedef struct { ULONG format_version; ULONG assembly_cnt_min; @@ -2860,6 +2904,28 @@ todo_wine { } } +static void test_CreateActCtx_share_mode(void) +{ + WCHAR tmp_manifest_pathname[MAX_PATH]; + HANDLE handle, manifest_file; + ACTCTXW actctx; + + manifest_file = create_temp_manifest_file(manifest1, tmp_manifest_pathname); + + memset(&actctx, 0, sizeof(ACTCTXW)); + actctx.cbSize = sizeof(ACTCTXW); + actctx.lpSource = tmp_manifest_pathname; + + handle = CreateActCtxW(&actctx); + todo_wine + ok(handle != INVALID_HANDLE_VALUE, "CreateActCtxW returned error %lu\n", GetLastError()); + ok(handle != NULL, "CreateActCtxW returned %p\n", handle); + + ReleaseActCtx(handle); + + CloseHandle(manifest_file); +} + static BOOL init_funcs(void) { HMODULE hLibrary = GetModuleHandleA("kernel32.dll"); @@ -3719,6 +3785,7 @@ START_TEST(actctx) test_actctx(); test_create_fail(); test_CreateActCtx(); + test_CreateActCtx_share_mode(); test_findsectionstring(); test_ZombifyActCtx(); run_child_process(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2433
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Today, RtlCreateActivationContext (CreateActCtxW) opens the source manifest file via NtOpenFile without the FILE_SHARE_DELETE sharing mode. This causes CreateActCtxW to fail if the source manifest file was created with the FILE_DELETE_ON_CLOSE flag. FILE_DELETE_ON_CLOSE is often used for temporary files that should be automatically deleted after use, even if the creator process crashes. Fix this by specifying FILE_SHARE_DELETE for sharing mode when opening the source manifest or module file. This allows the source manifest or module file to be marked as deleted while it is open. Note that concurrent deletion is not an issue for the following reasons: - The ability to read from an open file handle is unaffected by deletion of the corresponding file's name. - RtlCreateActivationContext does not open the source manifest or module file by the given filename (lpSource) more than once. --- dlls/kernel32/tests/actctx.c | 1 - dlls/ntdll/actctx.c | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index 9dbf77bc761..da351e70466 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -2917,7 +2917,6 @@ static void test_CreateActCtx_share_mode(void) actctx.lpSource = tmp_manifest_pathname; handle = CreateActCtxW(&actctx); - todo_wine ok(handle != INVALID_HANDLE_VALUE, "CreateActCtxW returned error %lu\n", GetLastError()); ok(handle != NULL, "CreateActCtxW returned %p\n", handle); diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index 721a2f339a5..b275a7f5b49 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -2925,7 +2925,8 @@ static NTSTATUS open_nt_file( HANDLE *handle, UNICODE_STRING *name ) attr.ObjectName = name; attr.SecurityDescriptor = NULL; attr.SecurityQualityOfService = NULL; - return NtOpenFile( handle, GENERIC_READ | SYNCHRONIZE, &attr, &io, FILE_SHARE_READ, FILE_SYNCHRONOUS_IO_ALERT ); + return NtOpenFile( handle, GENERIC_READ | SYNCHRONIZE, &attr, &io, + FILE_SHARE_READ | FILE_SHARE_DELETE, FILE_SYNCHRONOUS_IO_ALERT ); } static NTSTATUS get_manifest_in_module( struct actctx_loader* acl, struct assembly_identity* ai, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2433
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=130802 Your paranoid android. === debian11 (32 bit report) === d3d9: device: Timeout
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2433
participants (4)
-
Jacek Caban (＠jacek) -
Jinoh Kang
-
Jinoh Kang (＠iamahuman)
-
Marvin