[PATCH 2/2 v2] ntdll: Track copy-on-write memory state on i386 and x86_64 architectures.

[Sebastian Lackner]

Hello Piotr,

On 22.02.19 17:38, Piotr Caban wrote:
> Hi Sebastian,
> 
> On 2/22/19 5:08 PM, Sebastian Lackner wrote:
>> I didn't fully review your patch, but note that we had this feature
>> basically disabled in Staging (hidden behind an environment variable)
>> because it caused a lot of trouble.
>>
>> The main issues were:
>>
>> * Syscalls will just fail with EFAULT when they encounter a page
>> without sufficient protections. It will not trigger a signal! This
>> means it would be necessary to add code to handle EFAULT whenever
>> there is a chance that the memory passed by the user might have the
>> copy-on-write flag. In particular, this affects all wineserver calls
>> which directly write to user-provided buffers. See:
>>
>> https://github.com/wine-staging/wine-staging/blob/master/patches/ntdll-WRITECOPY/0001-ntdll-Trigger-write-watches-before-passing-userdata-.patch 
>>
> The code is changed in a way so it behaves exactly the same as memory 
> with write watch. Before executing the syscall check_write_access should 
> make the memory readable. I think copy-on-write EFAULT shouldn't happen 
> during syscall in current wine.

When check_write_access() is called everything should be fine, but it 
doesn't look like it is used everywhere. What about all the NtQuery*() 
functions that pass through user-provided pointers? See for example 
NtQueryInformationAtom or RtlQueryAtomInAtomTable - both directly pass 
an untrusted pointer to wine_server_set_reply() and both use 
wine_server_call() instead of any of the locked variants. Also, there 
still seems to be at least one plain read() call in advapi32:

https://github.com/wine-staging/wine-staging/blob/master/patches/ntdll-WRITECOPY/0002-advapi-Trigger-write-watches-before-passing-userdata.patch

> 
>> * For third party libraries you always have to ensure that faults are
>> handled before passing any pointer. This even affects the OpenGL libs:
>> They pass memory addresses directly to the kernel, and thus don't
>> trigger the write patches. We noticed weird rendering errors in
>> several games with the copy-on-write logic enabled.
> Do you remember any of the games that were affected? I'm expecting it to 
> still be a problem.

Unfortunately I am not sure anymore which game showed the graphic 
issues, but I would suspect that even a simple OpenGL sample code to 
download a texture into WRITECOPY memory would be affected. There were 
at least two bug reports related to WRITECOPY regressions though:

* https://bugs.winehq.org/show_bug.cgi?id=39349
* https://bugs.winehq.org/show_bug.cgi?id=39350

For the second one the app has probably changed in the meantime, but 
maybe the first one can still be used to reproduce one of the issues.

Best regards,
Sebastian