[PATCH v20 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. -- v20: 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..f0105d83f93 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 = allocation; + 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 = pUnmapViewOfFile2(GetCurrentProcess(), view, MEM_PRESERVE_PLACEHOLDER); + ok(ret, "UnmapViewOfFile2 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 01:02:28 2024 +0000, Felix Münchhalfen wrote:
Oh! To my shame i must admit i tested this with wine-staging patches applied. There is one patch in there that modifies how views are handled. But anyways, i fixed the tests now, and i believe using VirtualFree in that way is more correct. I was just curious before, if it would be possible to split a big placeholder allocation into two, with a hole in the middle (this works with wine-staging, but not in dev). But when i think about it, i dont think that this is how it's supposed to work. You would rather have to start chewing off of a big placeholder from the start to the end. Please test again now. I tested with both pure wine and with staging patches. The tests work in both. 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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4822#note_59567
participants (2)
-
Felix Münchhalfen -
Paul Gofman (＠gofman)