[PATCH 0/3] MR6767: WriteProcessMemory() without PROCESS_QUERY_INFORMATION access right.
This is an alternate solution to MR 6761 [1] (actually is based on comment in that MR). WriteProcessMemory() should succeed when process handle doesn't have PROCESS_QUERY_INFORMATION access right set (test attached). Plus some additional cleanup in test for ReadProcessMemory(). Feedback from Proton says that this patch solves the reported game issue. [1] https://gitlab.winehq.org/wine/wine/-/merge_requests/6761 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6767
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/virtual.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c index 12841133c05..cfbfcac0950 100644 --- a/dlls/kernel32/tests/virtual.c +++ b/dlls/kernel32/tests/virtual.c @@ -4368,13 +4368,15 @@ static void test_PrefetchVirtualMemory(void) static void test_ReadProcessMemory(void) { - BYTE buf[0x2000]; + BYTE *buf; DWORD old_prot; SIZE_T copied; HANDLE hproc; void *ptr; BOOL ret; + buf = malloc(2 * si.dwPageSize); + ok(buf != NULL, "OOM\n"); ret = DuplicateHandle(GetCurrentProcess(), GetCurrentProcess(), GetCurrentProcess(), &hproc, 0, FALSE, DUPLICATE_SAME_ACCESS); ok(ret, "DuplicateHandle failed %lu\n", GetLastError()); @@ -4403,6 +4405,7 @@ static void test_ReadProcessMemory(void) ret = VirtualFree(ptr, 0, MEM_RELEASE); ok(ret, "VirtualFree failed %lu\n", GetLastError()); CloseHandle(hproc); + free(buf); } START_TEST(virtual) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6767
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/virtual.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c index cfbfcac0950..0feac0d70ac 100644 --- a/dlls/kernel32/tests/virtual.c +++ b/dlls/kernel32/tests/virtual.c @@ -4408,6 +4408,32 @@ static void test_ReadProcessMemory(void) free(buf); } +static void test_WriteProcessMemory(void) +{ + BYTE src[32], dst[sizeof(src)]; + HANDLE hproc; + SIZE_T written; + BOOL ret; + + /* test minimum process handle access rights */ + ret = DuplicateHandle(GetCurrentProcess(), GetCurrentProcess(), GetCurrentProcess(), + &hproc, PROCESS_VM_OPERATION | PROCESS_VM_WRITE, FALSE, 0); + ok(ret, "DuplicateHandle failed %lu\n", GetLastError()); + + memset(src, 0xA5, sizeof(src)); + memset(dst, 0x5A, sizeof(dst)); + written = 666; + ret = WriteProcessMemory(hproc, dst, src, sizeof(src), &written); + todo_wine + ok(ret, "WriteProcessMemory failed\n"); + todo_wine + ok(written == sizeof(src), "copied = %Id\n", written); + todo_wine + ok(dst[0] == 0xA5 && dst[sizeof(dst) - 1] == 0xA5, "Write operation failed\n"); + + CloseHandle(hproc); +} + START_TEST(virtual) { int argc; @@ -4490,6 +4516,7 @@ START_TEST(virtual) test_write_watch(); test_PrefetchVirtualMemory(); test_ReadProcessMemory(); + test_WriteProcessMemory(); #if defined(__i386__) || defined(__x86_64__) test_stack_commit(); #endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6767
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/kernel32/tests/virtual.c | 3 --- dlls/kernelbase/memory.c | 17 +++++++++++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c index 0feac0d70ac..f8700473163 100644 --- a/dlls/kernel32/tests/virtual.c +++ b/dlls/kernel32/tests/virtual.c @@ -4424,11 +4424,8 @@ static void test_WriteProcessMemory(void) memset(dst, 0x5A, sizeof(dst)); written = 666; ret = WriteProcessMemory(hproc, dst, src, sizeof(src), &written); - todo_wine ok(ret, "WriteProcessMemory failed\n"); - todo_wine ok(written == sizeof(src), "copied = %Id\n", written); - todo_wine ok(dst[0] == 0xA5 && dst[sizeof(dst) - 1] == 0xA5, "Write operation failed\n"); CloseHandle(hproc); diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c index 10745303499..83eaa70507a 100644 --- a/dlls/kernelbase/memory.c +++ b/dlls/kernelbase/memory.c @@ -612,8 +612,21 @@ BOOL WINAPI DECLSPEC_HOTPATCH WriteProcessMemory( HANDLE process, void *addr, co if (!VirtualQueryEx( process, addr, &info, sizeof(info) )) { - close_cross_process_connection( list ); - return FALSE; + HANDLE process_alt; + BOOL alt_ok = FALSE; + + if (GetLastError() == ERROR_ACCESS_DENIED && + DuplicateHandle( GetCurrentProcess(), process, GetCurrentProcess(), &process_alt, + PROCESS_QUERY_INFORMATION, FALSE, 0 )) + { + alt_ok = VirtualQueryEx( process_alt, addr, &info, sizeof(info) ); + CloseHandle( process_alt ); + } + if (!alt_ok) + { + close_cross_process_connection( list ); + return FALSE; + } } switch (info.Protect & ~(PAGE_GUARD | PAGE_NOCACHE)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6767
We should first make sure that NtQueryVirtualMemory behaves correctly. A quick test suggests that it should succeed for PROCESS_VM_OPERATION|PROCESS_VM_WRITE. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6767#note_86538
participants (3)
-
Alexandre Julliard (＠julliard) -
Eric Pouech
-
eric pouech (＠epo)