[PATCH v2 0/1] 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. -- v2: ntdll: Fix: Ignore alignment if MEM_REPLACE_PLACEHOLDER is set in flags 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, 4 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index abe1b4dc4ec..d8d04009248 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5466,7 +5466,8 @@ NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_p } #endif - if ((offset.u.LowPart & mask) || (*addr_ptr && ((UINT_PTR)*addr_ptr & mask))) + if (!(alloc_type & MEM_REPLACE_PLACEHOLDER) && + ((offset.u.LowPart & mask) || (*addr_ptr && ((UINT_PTR)*addr_ptr & mask)))) return STATUS_MAPPED_ALIGNMENT; if (process != NtCurrentProcess()) @@ -5535,7 +5536,8 @@ NTSTATUS WINAPI NtMapViewOfSectionEx( HANDLE handle, HANDLE process, PVOID *addr } #endif - if ((offset.u.LowPart & mask) || (*addr_ptr && ((UINT_PTR)*addr_ptr & mask))) + if (!(alloc_type & MEM_REPLACE_PLACEHOLDER) && + ((offset.u.LowPart & mask) || (*addr_ptr && ((UINT_PTR)*addr_ptr & mask)))) return STATUS_MAPPED_ALIGNMENT; if (process != NtCurrentProcess()) -- 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=141714 Your paranoid android. === 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 20:15:31 2024 +0000, Felix Münchhalfen wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/4822/diffs?diff_id=92796&start_sha=617ad81792fa873207ee5a1b6d4a73042af26863#584f4313ed133393a8f1903c21dcaf4967ba7ab9_5539_5539) This is picky. i force pushed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4822#note_57079
On Wed Jan 10 20:39:21 2024 +0000, Nikolay Sivov wrote:
Thank you for the MR. We'll definitely need a test for this, preferably for both the syscall and user wrapper in kernel32. Yes, and ideally the test would actually map something to see how that works and confirm what is the actual align requirement (I'd guess the addresses should still be at least page aligned and that still should be checked?).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4822#note_57082
On Wed Jan 10 20:39:21 2024 +0000, Paul Gofman wrote:
Yes, and ideally the test would actually map something to see how that works and confirm what is the actual align requirement (I'd guess the addresses should still be at least page aligned and that still should be checked?). @nsivov @gofman :
I dont know, i can change it so that it must be page aligned. But msdn just says there are no requirements on the alignment if the flag is there. I made something as a ground for a test, maybe you can polish it up [testcase.txt](/uploads/adc1bb368edb696a3e6d44a76117dea2/testcase.txt) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4822#note_57089
On Wed Jan 10 21:05:06 2024 +0000, Felix Münchhalfen wrote:
@nsivov @gofman : I dont know, i can change it so that it must be page aligned. But msdn just says there are no requirements on the alignment if the flag is there. I made something as a ground for a test, maybe you can polish it up [testcase.txt](/uploads/adc1bb368edb696a3e6d44a76117dea2/testcase.txt) You can test it with windows (it runs) and with current wine (it does not run).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4822#note_57090
I made something as a ground for a test, maybe you can polish it up
maybe, but FWIW establishing how exactly that works and making proper test is the most of work here, tweaking the validation in the code to make an app happy is less of a hastle and was already done by Dmirty here: https://bugs.winehq.org/show_bug.cgi?id=56122#c1 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4822#note_57091
On Wed Jan 10 21:06:01 2024 +0000, Paul Gofman wrote:
I made something as a ground for a test, maybe you can polish it up maybe, but FWIW establishing how exactly that works and making proper test is the most of work here, tweaking the validation in the code to make an app happy is less of a hastle and was already done by Dmirty here: https://bugs.winehq.org/show_bug.cgi?id=56122#c1 tweaking the validation according to msdn.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4822#note_57093
On Wed Jan 10 21:12:30 2024 +0000, Felix Münchhalfen wrote:
tweaking the validation according to msdn. @gofman your suspicion is right. if they are not pagesize aligned it fails on windows too. (with ERROR_MAPPED_ALIGNMENT)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4822#note_57094
participants (3)
-
Felix Münchhalfen -
Marvin -
Paul Gofman (＠gofman)