[PATCH v22 0/2] MR4822: ntdll: Fix: Use pagesize alignment if MEM_REPLACE_PLACEHOLDER is set in flags of NtMapViewOfSection(Ex)
I had a bugreport here: https://bugs.winehq.org/show_bug.cgi?id=56161 This pull req fixes the bug that programs that do VirtualAlloc(placeholder)/VirtualFree(keep placeholder)/MapViewOfFile3(replace placeholder), do not run. Like the dotnet pe loader in .net 7 for example. It was not clear to me at first, because i didnt notice it on msdn, but the way that Dmitry Timoshkov "hacked" it in https://bugs.winehq.org/show_bug.cgi?id=56122 is actually (nearly) how it is supposed to happen according to msdn.
From here: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-m... 
So thanks to Dmitry Timoshkov. If you are interested you could also look into the thing i mentioned in the bug report, that MapViewOfFile3 doesn't round down to 64k, but, i don't think this is a serious problem yet. Edit: I found out that under windows, the alignment constraint is not fully lifted but, it is only required to be pagesize aligned if MEM_REPLACE_PLACEHOLDER is present in the flags. To clarify why i want this in wine. The PE Loader of .net (see here https://github.com/dotnet/runtime/blob/f21dc6c3dceb6ea76bef73e2a026c770aaed3...) does align with page size. Not with 64k. This breaks a .net 7 app during load up, that will execute otherwise just execute fine. -- v22: kernelbase: Added a test for MapViewOfFile3 with MEM_REPLACE_PLACEHOLDER https://gitlab.winehq.org/wine/wine/-/merge_requests/4822
From: Felix Münchhalfen<jan.felix.muenchhalfen(a)rwth-aachen.de> --- dlls/ntdll/unix/virtual.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index abe1b4dc4ec..2a53f56879e 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5466,6 +5466,9 @@ NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_p } #endif + if (alloc_type & MEM_REPLACE_PLACEHOLDER) + mask = page_mask; + if ((offset.u.LowPart & mask) || (*addr_ptr && ((UINT_PTR)*addr_ptr & mask))) return STATUS_MAPPED_ALIGNMENT; @@ -5535,6 +5538,9 @@ NTSTATUS WINAPI NtMapViewOfSectionEx( HANDLE handle, HANDLE process, PVOID *addr } #endif + if (alloc_type & MEM_REPLACE_PLACEHOLDER) + mask = page_mask; + if ((offset.u.LowPart & mask) || (*addr_ptr && ((UINT_PTR)*addr_ptr & mask))) return STATUS_MAPPED_ALIGNMENT; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4822
From: Felix Münchhalfen<jan.felix.muenchhalfen(a)rwth-aachen.de> --- dlls/kernelbase/tests/process.c | 83 +++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/dlls/kernelbase/tests/process.c b/dlls/kernelbase/tests/process.c index 88a527c45d5..6fab412ce79 100644 --- a/dlls/kernelbase/tests/process.c +++ b/dlls/kernelbase/tests/process.c @@ -106,6 +106,12 @@ static void test_MapViewOfFile3(void) HANDLE file, mapping; void *ptr; BOOL ret; + SYSTEM_INFO system_info; + DWORD error; + size_t file_size = 1024UL*1024; /* 1M */ + void *allocation; + void *map_start, *map_end, *map_start_offset; + void *view; if (!pMapViewOfFile3) { @@ -132,6 +138,83 @@ static void test_MapViewOfFile3(void) CloseHandle( file ); ret = DeleteFileA( testfile ); ok(ret, "Failed to delete a test file.\n"); + + /* Tests for using MapViewOfFile3 together with MEM_RESERVE_PLACEHOLDER/MEM_REPLACE_PLACEHOLDER */ + /* like self pe-loading programs do (e.g. .net pe-loader). */ + /* With MEM_REPLACE_PLACEHOLDER, MapViewOfFile3/NtMapViewOfSection(Ex) shall relax alignment from 64k to pagesize */ + GetSystemInfo(&system_info); + mapping = CreateFileMappingA(NULL, NULL, PAGE_READWRITE, 0, (DWORD)file_size, NULL); + ok(mapping != NULL, "CreateFileMapping did not return a handle %lu\n", GetLastError()); + + allocation = pVirtualAlloc2(GetCurrentProcess(), NULL, file_size, + MEM_RESERVE | MEM_RESERVE_PLACEHOLDER, PAGE_NOACCESS, NULL, 0); + ok(allocation != NULL, "VirtualAlloc2 returned NULL %lu\n", GetLastError()); + + map_start = (void*)(((ULONG_PTR)allocation + system_info.dwPageSize) & ~((ULONG_PTR)system_info.dwPageSize-1));; + map_end = (void*)((ULONG_PTR)map_start + system_info.dwPageSize); + ret = VirtualFree(map_start, system_info.dwPageSize, MEM_RELEASE | MEM_PRESERVE_PLACEHOLDER); + ok(ret, "VirtualFree failed to split the placeholder %lu\n", GetLastError()); + + view = pMapViewOfFile3(mapping, GetCurrentProcess(), map_start, system_info.dwPageSize, + system_info.dwPageSize, MEM_REPLACE_PLACEHOLDER, PAGE_READWRITE, NULL, 0); + ok(view != NULL, "MapViewOfFile3 did not map the file mapping %lu\n", GetLastError()); + + ret = UnmapViewOfFile(view); + ok(ret, "UnmapViewOfFile failed %lu\n", GetLastError()); + + map_start_offset = (void*)((ULONG_PTR)map_start - 1); + view = pMapViewOfFile3(mapping, GetCurrentProcess(), map_start_offset, system_info.dwPageSize, + system_info.dwPageSize, MEM_REPLACE_PLACEHOLDER, PAGE_READWRITE, NULL, 0); + ok(view == NULL, "MapViewOfFile3 did map the file mapping, though baseaddr was not pagesize aligned\n"); + error = GetLastError(); + ok(error == ERROR_MAPPED_ALIGNMENT, "MapViewOfFile3 did not return ERROR_MAPPED_ALIGNMENT(%u), instead it returned %lu\n", + ERROR_MAPPED_ALIGNMENT, error); + + view = pMapViewOfFile3(mapping, GetCurrentProcess(), map_start, system_info.dwPageSize-1, + system_info.dwPageSize, MEM_REPLACE_PLACEHOLDER, PAGE_READWRITE, NULL, 0); + ok(view == NULL, "MapViewOfFile3 did map the file mapping, though offset was not pagesize aligned\n"); + error = GetLastError(); + ok(error == ERROR_MAPPED_ALIGNMENT, + "MapViewOfFile3 did not return ERROR_MAPPED_ALIGNMENT(%u), instead it returned %lu\n", + ERROR_MAPPED_ALIGNMENT, error); + + ret = VirtualFree(allocation, 0, MEM_RELEASE); + ok(ret, "VirtualFree of first remaining region failed: %lu\n", GetLastError()); + + ret = VirtualFree(map_end, 0, MEM_RELEASE); + ok(ret, "VirtualFree of remaining region failed: %lu\n", GetLastError()); + + view = pMapViewOfFile3(mapping, GetCurrentProcess(), map_end, system_info.dwPageSize, 0, 0, PAGE_READWRITE, NULL, 0); + ok(view == NULL, "MapViewOfFile3 did map the file mapping, though baseaddr was not 64k aligned\n"); + error = GetLastError(); + ok(error == ERROR_MAPPED_ALIGNMENT, + "MapViewOfFile3 did not return ERROR_MAPPED_ALIGNMENT(%u), instead it returned %lu\n", + ERROR_MAPPED_ALIGNMENT, error); + + map_start = (void*)(((ULONG_PTR)allocation + system_info.dwAllocationGranularity)&~(system_info.dwAllocationGranularity-1)); + view = pMapViewOfFile3(mapping, GetCurrentProcess(), map_end, + system_info.dwPageSize, system_info.dwPageSize, 0, PAGE_READWRITE, NULL, 0); + ok(view == NULL, "MapViewOfFile3 did map the file mapping, though offset was not 64k aligned\n"); + error = GetLastError(); + ok(error == ERROR_MAPPED_ALIGNMENT, + "MapViewOfFile3 did not return ERROR_MAPPED_ALIGNMENT(%u), instead it returned %lu\n", + ERROR_MAPPED_ALIGNMENT, error); + + view = pMapViewOfFile3(mapping, GetCurrentProcess(), map_start, 0, + system_info.dwPageSize, 0, PAGE_READWRITE, NULL, 0); + ok(view != NULL, "MapViewOfFile3 failed though both baseaddr and offset were 64k aligned %lu\n", GetLastError()); + + ret = UnmapViewOfFile(view); + ok(ret, "UnmapViewOfFile failed %lu\n", GetLastError()); + + view = pMapViewOfFile3(mapping, GetCurrentProcess(), map_start, + 4*system_info.dwAllocationGranularity, system_info.dwPageSize, 0, PAGE_READWRITE, NULL, 0); + ok(view != NULL, "MapViewOfFile3 failed though both baseaddr and offset were 64k aligned %lu\n", GetLastError()); + + ret = UnmapViewOfFile(view); + ok(ret, "UnmapViewOfFile failed %lu\n", GetLastError()); + + ok(CloseHandle(mapping), "CloseHandle failed on mapping\n"); } #define check_region_size(p, s) check_region_size_(p, s, __LINE__) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4822
On Wed Jan 31 02:12:33 2024 +0000, Paul Gofman wrote:
VirtualFree supports splitting placeholders in Wine, including in the middle of the view like in your test. The actual problem in your patch is that ```map_start = (void*)(((ULONG_PTR)allocation + system_info.dwPageSize) & ~(system_info.dwPageSize-1));``` truncates the address wtih & part, should be ```map_start = (void*)(((ULONG_PTR)allocation + system_info.dwPageSize) & ~((ULONG_PTR)system_info.dwPageSize-1));``` or you can actually drop that alignment at all, allocation + page_size is already aligned. Why it works on Windows (and with some Wine-Staging patches) is because the memory is allocated top-down and you happen to get the address below 4gb so truncation doesn't matter. I suggest to keep your test as it was with mapping in the middle of the initial view, it is probably a bit more interesting this way, fixing just the truncation part. Thank you very much for the hint. I missed casting it to 64bit :smiley:
I changed it back with your correction and now the tests work, as i wanted them to be. Eating away a piece in the middle of the placeholded allocation. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4822#note_59731
participants (1)
-
Felix Münchhalfen