[PATCH v5 0/2] MR4822: ntdll: Fix: Ignore 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 exactly 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. -- v5: 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 | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/dlls/kernelbase/tests/process.c b/dlls/kernelbase/tests/process.c index 88a527c45d5..1ca6792bc1d 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 *pVirtualFree)(LPVOID,SIZE_T,DWORD); 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); @@ -134,6 +135,37 @@ static void test_MapViewOfFile3(void) ok(ret, "Failed to delete a test file.\n"); } +static void test_MapViewOfFileReplacePlaceholder(void) +{ + SYSTEM_INFO si; + size_t szFile = 1024UL*1024; // 1M + HANDLE hFileMapping; + PVOID pAllocation; + PVOID pMapStart; + PVOID pMapEnd; + BOOL bVirtualFree; + PVOID pMappedFileView; + + GetSystemInfo(&si); + + hFileMapping = CreateFileMappingA(NULL, NULL, PAGE_READWRITE, 0, (DWORD)szFile, NULL); + ok(hFileMapping != NULL, "CreateFileMapping did not return a handle\n"); + + pAllocation = pVirtualAlloc2(GetCurrentProcess(), NULL, szFile, MEM_RESERVE | MEM_RESERVE_PLACEHOLDER, PAGE_NOACCESS, NULL, 0); + ok(pAllocation != NULL, "VirtualAlloc2 returned NULL\n"); + + pMapStart = (void*)(((unsigned long long)pAllocation + si.dwPageSize) & ~(si.dwPageSize-1)); // advance forward to align with pagesize + pMapEnd = (void*)(((unsigned long long)pAllocation + 2*si.dwPageSize) & ~(si.dwPageSize-1)); + bVirtualFree = pVirtualFree(pMapStart, si.dwPageSize, MEM_RELEASE | MEM_PRESERVE_PLACEHOLDER); // split the region into two placeholders + ok(bVirtualFree, "VirtualFree failed to split the placeholder\n"); + pMappedFileView = pMapViewOfFile3(hFileMapping, GetCurrentProcess(), pMapStart, si.dwPageSize, si.dwPageSize, MEM_REPLACE_PLACEHOLDER, PAGE_READWRITE, NULL, 0); + ok(pMappedFileView != NULL, "MapViewOfFile3 did not map the FileMapping\n"); + + pVirtualFree(pMapEnd, szFile - 2*si.dwPageSize, MEM_RELEASE); + ok(bVirtualFree, "VirtualFree failed to free the remaining placeholder\n"); + ok(CloseHandle(hFileMapping), "CloseHandle failed on hFileMapping\n"); +} + #define check_region_size(p, s) check_region_size_(p, s, __LINE__) static void check_region_size_(void *p, SIZE_T s, unsigned int line) { @@ -519,6 +551,7 @@ static void init_funcs(void) X(VirtualAlloc2); X(VirtualAlloc2FromApp); X(VirtualAllocFromApp); + X(VirtualFree); X(UnmapViewOfFile2); hmod = GetModuleHandleA("ntdll.dll"); @@ -533,6 +566,7 @@ START_TEST(process) test_CompareObjectHandles(); test_MapViewOfFile3(); + test_MapViewOfFileReplacePlaceholder(); test_VirtualAlloc2(); test_VirtualAllocFromApp(); test_VirtualAlloc2FromApp(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4822
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=141718 Your paranoid android. === w7u_2qxl (32 bit report) === kernelbase: 0558:process: unhandled exception c0000005 at 00000000 === w7u_adm (32 bit report) === kernelbase: 0860:process: unhandled exception c0000005 at 00000000 === w7u_el (32 bit report) === kernelbase: 0a60:process: unhandled exception c0000005 at 00000000 === w8 (32 bit report) === kernelbase: 0bd8:process: unhandled exception c0000005 at 00000000 === w8adm (32 bit report) === kernelbase: 0ab4:process: unhandled exception c0000005 at 00000000 === w864 (32 bit report) === kernelbase: 0564:process: unhandled exception c0000005 at 00000000 === w1064v1507 (32 bit report) === kernelbase: 0634:process: unhandled exception c0000005 at 00000000 === w7pro64 (64 bit report) === kernelbase: 0bac:process: unhandled exception c0000005 at 0000000000000000 === w864 (64 bit report) === kernelbase: 0b30:process: unhandled exception c0000005 at 0000000000000000 === w1064v1507 (64 bit report) === kernelbase: 0f64:process: unhandled exception c0000005 at 0000000000000000 === debian11b (64 bit WoW report) === wininet: http.c:6820: Test failed: lpszSubjectInfo = winehq.org http.c:6821: Test failed: lpszIssuerInfo = US
On Wed Jan 10 22:15:02 2024 +0000, Felix Münchhalfen wrote:
I am currently writing a test for memory.c in kernelbase.dll for this case. Just one sec =) Sorry i didn't understand at first what "test" meant. Its in there now.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4822#note_57120
participants (2)
-
Felix Münchhalfen -
Marvin