[PATCH v2 0/1] MR4822: ntdll: Fix: Ignore alignment if MEM_REPLACE_PLACEHOLDER is set in flags of NtMapViewOfSection(Ex)

[Felix Münchhalfen (＠jfmu)]

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...
![image](/uploads/58614927d38c15d4c23517aa5dc09d77/image.png)

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

[PATCH v2 1/1] ntdll: Fix: Ignore alignment if MEM_REPLACE_PLACEHOLDER is set in flags

[Felix Münchhalfen]

From: Felix Münchhalfen

---
 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())

[PATCH v2 1/1] ntdll: Fix: Ignore alignment if MEM_REPLACE_PLACEHOLDER is set in flags

[Marvin]

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

[Felix Münchhalfen (＠jfmu)]

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.

[Paul Gofman (＠gofman)]

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?).

[Felix Münchhalfen (＠jfmu)]

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)

[Felix Münchhalfen (＠jfmu)]

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).

[Paul Gofman (＠gofman)]

> 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

[Felix Münchhalfen (＠jfmu)]

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.

[Felix Münchhalfen (＠jfmu)]

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)