[PATCH v8 0/4] MR8553: kernelbase: implement VirtualProtectFromApp
This implements the VirtualProtectFromApp function in kernelbase, which was previously a commented out stub. Even though this function is meant for UWP apps, it is usable in desktop apps, as specified in [Windows documentation](https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-v...). Fixes Wine-Bug: [https://bugs.winehq.org/show_bug.cgi?id=58482](https://bugs.winehq.org/show_bug.cgi?id=58482) -- v8: ntdll/tests: Add tests for NtProtectVirtualMemory() ntdll: Set old_prot to PAGE_NOACCESS in NtProtectVirtualMemory() if the range is not mapped or committed. https://gitlab.winehq.org/wine/wine/-/merge_requests/8553
From: Kareem Aladli <karimri(a)protonmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58482 Signed-off-by: Kareem Aladli <karimri(a)protonmail.com> --- dlls/kernelbase/kernelbase.spec | 2 +- dlls/kernelbase/memory.c | 9 +++++++++ include/winbase.h | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/dlls/kernelbase/kernelbase.spec b/dlls/kernelbase/kernelbase.spec index 545c30a1eb8..48471834735 100644 --- a/dlls/kernelbase/kernelbase.spec +++ b/dlls/kernelbase/kernelbase.spec @@ -1729,7 +1729,7 @@ @ stdcall VirtualLock(ptr long) @ stdcall VirtualProtect(ptr long long ptr) @ stdcall VirtualProtectEx(long ptr long long ptr) -# @ stub VirtualProtectFromApp +@ stdcall VirtualProtectFromApp(ptr long long ptr) @ stdcall VirtualQuery(ptr ptr long) @ stdcall VirtualQueryEx(long ptr ptr long) @ stdcall VirtualUnlock(ptr long) diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c index 9bf8e291cbe..34c83eb428b 100644 --- a/dlls/kernelbase/memory.c +++ b/dlls/kernelbase/memory.c @@ -572,6 +572,15 @@ BOOL WINAPI DECLSPEC_HOTPATCH VirtualProtectEx( HANDLE process, void *addr, SIZE return set_ntstatus( NtProtectVirtualMemory( process, &addr, &size, new_prot, old_prot )); } +/*********************************************************************** + * VirtualProtectFromApp (kernelbase.@) + */ +BOOL WINAPI DECLSPEC_HOTPATCH VirtualProtectFromApp( void *addr, SIZE_T size, + ULONG new_prot, ULONG *old_prot ) +{ + /* Contrary to the documentation, VirtualProtectFromApp allows write+execute on desktop. */ + return VirtualProtect( addr, size, new_prot, old_prot ); +} /*********************************************************************** * VirtualQuery (kernelbase.@) diff --git a/include/winbase.h b/include/winbase.h index 29a3ea8fbce..b4fae70dda1 100644 --- a/include/winbase.h +++ b/include/winbase.h @@ -2479,6 +2479,7 @@ WINBASEAPI BOOL WINAPI VirtualFreeEx(HANDLE,LPVOID,SIZE_T,DWORD); WINBASEAPI BOOL WINAPI VirtualLock(LPVOID,SIZE_T); WINBASEAPI BOOL WINAPI VirtualProtect(LPVOID,SIZE_T,DWORD,LPDWORD); WINBASEAPI BOOL WINAPI VirtualProtectEx(HANDLE,LPVOID,SIZE_T,DWORD,LPDWORD); +WINBASEAPI BOOL WINAPI VirtualProtectFromApp(LPVOID,SIZE_T,ULONG,PULONG); WINBASEAPI SIZE_T WINAPI VirtualQuery(LPCVOID,PMEMORY_BASIC_INFORMATION,SIZE_T); WINBASEAPI SIZE_T WINAPI VirtualQueryEx(HANDLE,LPCVOID,PMEMORY_BASIC_INFORMATION,SIZE_T); WINBASEAPI BOOL WINAPI VirtualUnlock(LPVOID,SIZE_T); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8553
From: Kareem Aladli <karimri(a)protonmail.com> --- dlls/kernelbase/tests/process.c | 41 +++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/dlls/kernelbase/tests/process.c b/dlls/kernelbase/tests/process.c index 90d6b5204eb..0b1a83cfcca 100644 --- a/dlls/kernelbase/tests/process.c +++ b/dlls/kernelbase/tests/process.c @@ -38,6 +38,7 @@ static LPVOID (WINAPI *pMapViewOfFile3)(HANDLE, HANDLE, PVOID, ULONG64 offset, S static LPVOID (WINAPI *pVirtualAlloc2)(HANDLE, void *, SIZE_T, DWORD, DWORD, MEM_EXTENDED_PARAMETER *, ULONG); static LPVOID (WINAPI *pVirtualAlloc2FromApp)(HANDLE, void *, SIZE_T, DWORD, DWORD, MEM_EXTENDED_PARAMETER *, ULONG); static PVOID (WINAPI *pVirtualAllocFromApp)(PVOID, SIZE_T, DWORD, DWORD); +static BOOL (WINAPI *pVirtualProtectFromApp)(LPVOID,SIZE_T,ULONG,PULONG); static HANDLE (WINAPI *pOpenFileMappingFromApp)( ULONG, BOOL, LPCWSTR); static HANDLE (WINAPI *pCreateFileMappingFromApp)(HANDLE, PSECURITY_ATTRIBUTES, ULONG, ULONG64, PCWSTR); static LPVOID (WINAPI *pMapViewOfFileFromApp)(HANDLE, ULONG, ULONG64, SIZE_T); @@ -481,6 +482,44 @@ static void test_VirtualAlloc2FromApp(void) } } +static void test_VirtualProtectFromApp(void) +{ + ULONG old_prot; + void *p; + BOOL ret; + + if (!pVirtualProtectFromApp) + { + win_skip("VirtualProtectFromApp is not available.\n"); + return; + } + + SetLastError(0xdeadbeef); + p = VirtualAlloc(NULL, 0x1000, MEM_COMMIT, PAGE_READWRITE); + ok(p && GetLastError() == 0xdeadbeef, "Got unexpected mem %p, GetLastError() %lu.\n", p, GetLastError()); + ret = pVirtualProtectFromApp(p, 0x1000, PAGE_READONLY, &old_prot); + ok(ret && old_prot == PAGE_READWRITE, "Failed to change protection old_prot %lu, GetLastError() %lu\n", + old_prot, GetLastError()); + + ret = pVirtualProtectFromApp(p, 0x100000, PAGE_READWRITE, &old_prot); + ok(!ret && old_prot == PAGE_NOACCESS, "Call worked with overflowing size old_prot %lu\n", old_prot); + + ret = pVirtualProtectFromApp(p, 0x1000, PAGE_EXECUTE_READ, NULL); + ok(!ret, "Call worked without old_prot\n"); + + ret = pVirtualProtectFromApp(p, 0x1000, PAGE_GUARD, &old_prot); + /* Windows WoW64 sets old_prot to something bogus for an unknown reason */ + ok(!ret, "Call worked with an invalid new_prot parameter old_prot %lu\n", old_prot); + + /* Works on desktop, but not on UWP */ + ret = pVirtualProtectFromApp(p, 0x1000, PAGE_EXECUTE_READWRITE, &old_prot); + ok(ret && old_prot == PAGE_READONLY, "Failed to change protection old_prot %lu, GetLastError() %lu\n", + old_prot, GetLastError()); + + ret = VirtualFree(p, 0, MEM_RELEASE); + ok(ret, "Failed to free mem error %lu.\n", GetLastError()); +} + static void test_OpenFileMappingFromApp(void) { OBJECT_BASIC_INFORMATION info; @@ -601,6 +640,7 @@ static void init_funcs(void) X(VirtualAlloc2); X(VirtualAlloc2FromApp); X(VirtualAllocFromApp); + X(VirtualProtectFromApp); X(UnmapViewOfFile2); hmod = GetModuleHandleA("ntdll.dll"); @@ -618,6 +658,7 @@ START_TEST(process) test_VirtualAlloc2(); test_VirtualAllocFromApp(); test_VirtualAlloc2FromApp(); + test_VirtualProtectFromApp(); test_OpenFileMappingFromApp(); test_CreateFileMappingFromApp(); test_MapViewOfFileFromApp(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8553
From: Kareem Aladli <karimri(a)protonmail.com> --- dlls/ntdll/unix/virtual.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index e733e3ffdd6..c5408cbb5da 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5350,6 +5350,7 @@ NTSTATUS WINAPI NtProtectVirtualMemory( HANDLE process, PVOID *addr_ptr, SIZE_T if (!old_prot) return STATUS_ACCESS_VIOLATION; + *old_prot = PAGE_NOACCESS; if (process != NtCurrentProcess()) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8553
From: Kareem Aladli <karimri(a)protonmail.com> --- dlls/ntdll/tests/virtual.c | 72 ++++++++++++++++++++++++++++++++++++++ dlls/ntdll/unix/virtual.c | 19 +++++++--- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index c48652f0f65..0628a636264 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -2503,6 +2503,77 @@ static void test_NtFreeVirtualMemory(void) ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); } +static void test_NtProtectVirtualMemory(void) +{ + void *addr, *addr2; + NTSTATUS status; + SIZE_T size; + DWORD old_prot; + + size = page_size * 16; + addr = NULL; + status = NtAllocateVirtualMemory(NtCurrentProcess(), &addr, 0, &size, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + + old_prot = 0; + status = NtProtectVirtualMemory(NtCurrentProcess(), &addr, &size, PAGE_READONLY, &old_prot); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + ok(old_prot == PAGE_READWRITE, "Unexpected old_prot %lx.\n", old_prot); + + status = NtProtectVirtualMemory(NULL, &addr, &size, PAGE_READONLY, &old_prot); + ok(status == STATUS_INVALID_HANDLE, "Unexpected status %08lx.\n", status); + + status = NtProtectVirtualMemory(NtCurrentProcess(), &addr, &size, PAGE_READONLY, NULL); + ok(status == STATUS_ACCESS_VIOLATION, "Unexpected status %08lx.\n", status); + + status = NtProtectVirtualMemory(NtCurrentProcess(), &addr, &size, 0, &old_prot); + ok(status == STATUS_INVALID_PAGE_PROTECTION, "Unexpected status %08lx.\n", status); + + size = page_size * 8; + status = NtProtectVirtualMemory(NtCurrentProcess(), &addr, &size, PAGE_READWRITE, &old_prot); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + ok(old_prot == PAGE_READONLY, "Unexpected old_prot %lx.\n", old_prot); + addr = (char *)addr + page_size * 8; + status = NtProtectVirtualMemory(NtCurrentProcess(), &addr, &size, PAGE_EXECUTE_READWRITE, &old_prot); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + ok(old_prot == PAGE_READONLY, "Unexpected old_prot %lx.\n", old_prot); + addr = (char *)addr - page_size * 8; + size = page_size * 16; + status = NtProtectVirtualMemory(NtCurrentProcess(), &addr, &size, PAGE_EXECUTE_READ, &old_prot); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + ok(old_prot == PAGE_READWRITE, "Unexpected old_prot %lx.\n", old_prot); + + status = NtFreeVirtualMemory(NtCurrentProcess(), &addr, &size, MEM_RELEASE); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + + addr = NULL; + size = page_size; + status = NtProtectVirtualMemory(NtCurrentProcess(), &addr, &size, PAGE_READONLY, &old_prot); + /* todo: STATUS_INVALID_PARAMETER in wine, STATUS_CONFLICTING_ADDRESSES in win64 */ + ok(status, "Unexpected status %08lx.\n", status); + ok(old_prot == PAGE_NOACCESS, "Unexpected old_prot %lx.\n", old_prot); + + status = NtAllocateVirtualMemory(NtCurrentProcess(), &addr, 0, &size, MEM_RESERVE, PAGE_READWRITE); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + status = NtProtectVirtualMemory(NtCurrentProcess(), &addr, &size, PAGE_READONLY, &old_prot); + ok(status == STATUS_NOT_COMMITTED, "Unexpected status %08lx.\n", status); + ok(old_prot == PAGE_NOACCESS, "Unexpected old_prot %lx.\n", old_prot); + status = NtAllocateVirtualMemory(NtCurrentProcess(), &addr, 0, &size, MEM_COMMIT, PAGE_READWRITE); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + + addr2 = addr; + addr = (char *)addr + 1; + size = 1; + status = NtProtectVirtualMemory(NtCurrentProcess(), &addr, &size, PAGE_READONLY, &old_prot); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); + ok(old_prot == PAGE_READWRITE, "Unexpected old_prot %lx.\n", old_prot); + ok(size == page_size, "Unexpected size %p.\n", (void *)size); + ok(addr == addr2, "Got addr %p, addr2 %p.\n", addr, addr2); + + status = NtFreeVirtualMemory(NtCurrentProcess(), &addr, &size, MEM_RELEASE); + ok(status == STATUS_SUCCESS, "Unexpected status %08lx.\n", status); +} + static void test_prefetch(void) { NTSTATUS status; @@ -3251,6 +3322,7 @@ START_TEST(virtual) test_NtAllocateVirtualMemoryEx(); test_NtAllocateVirtualMemoryEx_address_requirements(); test_NtFreeVirtualMemory(); + test_NtProtectVirtualMemory(); test_RtlCreateUserStack(); test_NtMapViewOfSection(); test_NtMapViewOfSectionEx(); diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index c5408cbb5da..eb22cfa40df 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5350,7 +5350,6 @@ NTSTATUS WINAPI NtProtectVirtualMemory( HANDLE process, PVOID *addr_ptr, SIZE_T if (!old_prot) return STATUS_ACCESS_VIOLATION; - *old_prot = PAGE_NOACCESS; if (process != NtCurrentProcess()) { @@ -5366,11 +5365,15 @@ NTSTATUS WINAPI NtProtectVirtualMemory( HANDLE process, PVOID *addr_ptr, SIZE_T status = server_queue_process_apc( process, &call, &result ); if (status != STATUS_SUCCESS) return status; - if (result.virtual_protect.status == STATUS_SUCCESS) + switch (result.virtual_protect.status) { + case STATUS_SUCCESS: *addr_ptr = wine_server_get_ptr( result.virtual_protect.addr ); *size_ptr = result.virtual_protect.size; + case STATUS_NOT_COMMITTED: + case STATUS_INVALID_PARAMETER: *old_prot = result.virtual_protect.prot; + break; } return result.virtual_protect.status; } @@ -5390,9 +5393,17 @@ NTSTATUS WINAPI NtProtectVirtualMemory( HANDLE process, PVOID *addr_ptr, SIZE_T old = get_win32_prot( vprot, view->protect ); status = set_protection( view, base, size, new_prot ); } - else status = STATUS_NOT_COMMITTED; + else + { + status = STATUS_NOT_COMMITTED; + *old_prot = PAGE_NOACCESS; + } + } + else + { + status = STATUS_INVALID_PARAMETER; + *old_prot = PAGE_NOACCESS; } - else status = STATUS_INVALID_PARAMETER; if (!status) VIRTUAL_DEBUG_DUMP_VIEW( view ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8553
participants (2)
-
Kareem Aladli -
Kareem Aladli (＠karim_adli)