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