[Bug 37669] New: Resetting a write watch can cause memory access violation in kernel
https://bugs.winehq.org/show_bug.cgi?id=37669 Bug ID: 37669 Summary: Resetting a write watch can cause memory access violation in kernel Product: Wine Version: 1.7.32 Hardware: x86 OS: Linux Status: NEW Severity: normal Priority: P2 Component: -unknown Assignee: wine-bugs(a)winehq.org Reporter: dmitry(a)baikal.ru Distribution: --- What happens: heavy multi-threaded network application in one thread reads/writes from/to a socket while in another thread .net GC calls GetWriteWatch with flag WRITE_WATCH_FLAG_RESET on a buffer which was just verified for write access and passed by ws2_32.recv to recvmsg kernel call. While recvmsg waits for incoming data GetWriteWatch resets write watches by clearing VPROT_WRITE access on a buffer passed to recvmsg. That cases recvmsg to return EFAULT which in turn leads to a .net application raising an exception and dying. This is basically a .net garbage collector causing races in network code which reads/writes to a buffer because reading or writing via a network is much slower than say reading/writing a disk file, but essentially this is a general problem with resetting a write watch while another thread is being dealing with the affected memory range. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 Dmitry Timoshkov <dmitry(a)baikal.ru> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |focht(a)gmx.net --- Comment #1 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- AF might be interested :) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 Sebastian Lackner <sebastian(a)fds-team.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sebastian(a)fds-team.de -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #2 from Sebastian Lackner <sebastian(a)fds-team.de> --- If I understand you correctly we already have a patch for that: https://github.com/wine-compholio/wine-staging/blob/master/patches/ws2_32-Wr... It works on Windows because on Windows the kernel is responsible for handling write watches, not usermode. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #3 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #2)
If I understand you correctly we already have a patch for that: https://github.com/wine-compholio/wine-staging/blob/master/patches/ws2_32- WriteWatches/0001-ws2_32-Avoid-race-conditions-of-async-WSARecv-operat.patch
That patch fixes the problem, thanks. I have another workaround though: allocate a temporary buffer of the same size as passed to ws2_32 and use it for the recvmsg call, then just copy it to the application provided one. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #4 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #2)
If I understand you correctly we already have a patch for that: https://github.com/wine-compholio/wine-staging/blob/master/patches/ws2_32- WriteWatches/0001-ws2_32-Avoid-race-conditions-of-async-WSARecv-operat.patch
This approach may lead to infinite retries if recvmsg will legitimately fail for an invalid buffer due to not write watch related activities in another thread. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #5 from Sebastian Lackner <sebastian(a)fds-team.de> --- (In reply to Dmitry Timoshkov from comment #4)
(In reply to Sebastian Lackner from comment #2)
If I understand you correctly we already have a patch for that: https://github.com/wine-compholio/wine-staging/blob/master/patches/ws2_32- WriteWatches/0001-ws2_32-Avoid-race-conditions-of-async-WSARecv-operat.patch
This approach may lead to infinite retries if recvmsg will legitimately fail for an invalid buffer due to not write watch related activities in another thread.
I know, but your solution with copying memory has other disadvantages. I am not sure if allocating large amounts of memory (and then copying with an exception handler) is really better. Chances are relatively small that this will loop forever. Unfortunately there is no really perfect solution. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #6 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #5)
(In reply to Sebastian Lackner from comment #2)
If I understand you correctly we already have a patch for that: https://github.com/wine-compholio/wine-staging/blob/master/patches/ws2_32- WriteWatches/0001-ws2_32-Avoid-race-conditions-of-async-WSARecv-operat.patch
This approach may lead to infinite retries if recvmsg will legitimately fail for an invalid buffer due to not write watch related activities in another thread.
I know, but your solution with copying memory has other disadvantages. I am not sure if allocating large amounts of memory (and then copying with an exception handler) is really better. Chances are relatively small that this will loop forever. Unfortunately there is no really perfect solution.
There is no need for an exception handler, it works just fine without it. Regarding memory allocation: there are many other places in Wine that create intermediate buffers with the caller specified sizes. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #7 from Sebastian Lackner <sebastian(a)fds-team.de> --- (In reply to Dmitry Timoshkov from comment #6)
There is no need for an exception handler, it works just fine without it. Regarding memory allocation: there are many other places in Wine that create intermediate buffers with the caller specified sizes.
No, you'll need at least an SEH exception handler. On Windows its "valid" to release the memory while the async call is still pending or pass an invalid pointer to the winsock functions. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #8 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #7)
There is no need for an exception handler, it works just fine without it. Regarding memory allocation: there are many other places in Wine that create intermediate buffers with the caller specified sizes.
No, you'll need at least an SEH exception handler. On Windows its "valid" to release the memory while the async call is still pending or pass an invalid pointer to the winsock functions.
That's a minor problem IMHO, any other async operation or operation on a memory buffer that may be invalidated by another thread while the operation is in progress can cause this kind of a problem. But that situation should be found and analyzed first. The SEH handler just hides the bug and may lead to hardly diagnosabe bugs. It would be better IMO let it crash instead of silently eating an exception. Anyway that would qualify as an application bug IMHO. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #9 from Bruno Jesus <00cpxxx(a)gmail.com> --- Is it ok to set the component as winsock? Dmitry can you share your patch to solve this? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #10 from Sebastian Lackner <sebastian(a)fds-team.de> --- (In reply to Dmitry Timoshkov from comment #8)
That's a minor problem IMHO, any other async operation or operation on a memory buffer that may be invalidated by another thread while the operation is in progress can cause this kind of a problem. But that situation should be found and analyzed first. The SEH handler just hides the bug and may lead to hardly diagnosabe bugs. It would be better IMO let it crash instead of silently eating an exception. Anyway that would qualify as an application bug IMHO.
I don't agree, it could cause regressions. At the moment recvmsg just fails with EFAULT, but when you replace that with your suggested solution it will crash. As mentioned before, on Windows passing invalid pointers to WSA functions doesn't cause an exception. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #11 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #10)
I don't agree, it could cause regressions. At the moment recvmsg just fails with EFAULT, but when you replace that with your suggested solution it will crash.
recvmsg msg fails with EFAULT when it shouldn't, and that was very hard to track down. I'd prefer it would crash instead.
As mentioned before, on Windows passing invalid pointers to WSA functions doesn't cause an exception.
Neither under Wine. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #12 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Bruno Jesus from comment #9)
Is it ok to set the component as winsock?
I'd say that's an ntdll bug since write watches are implemented there.
Dmitry can you share your patch to solve this?
Once I find some time. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #13 from Sebastian Lackner <sebastian(a)fds-team.de> --- (In reply to Dmitry Timoshkov from comment #11)
(In reply to Sebastian Lackner from comment #10)
I don't agree, it could cause regressions. At the moment recvmsg just fails with EFAULT, but when you replace that with your suggested solution it will crash.
recvmsg msg fails with EFAULT when it shouldn't, and that was very hard to track down. I'd prefer it would crash instead.
Well, feel free to write a patch for your alternative solution, but I still think that checking for EFAULT, triggering write watches, and then retrying is a more clean approach.
As mentioned before, on Windows passing invalid pointers to WSA functions doesn't cause an exception.
Neither under Wine.
You should also keep in mind that there must be a reason why .NET / Silverlight uses write-watch protected pages for network buffers. I guess it is used as a fast method to determine when the async operation has been executed and new data is available. To make that fully compatible with Windows we should ideally get rid of the write watch check at the beginning of WSARecv, and only execute it when recvmsg returns EFAULT (see comments in my patch subject) - otherwise the application will check for completion multiple times. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #14 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #13)
You should also keep in mind that there must be a reason why .NET / Silverlight uses write-watch protected pages for network buffers. I guess it is used as a fast method to determine when the async operation has been executed and new data is available.
I'd guess that's a part of a GC (garbage collector). dotnet uses overlapped completion events to track down the end of a read/write operation.
To make that fully compatible with Windows we should ideally get rid of the write watch check at the beginning of WSARecv, and only execute it when recvmsg returns EFAULT (see comments in my patch subject) - otherwise the application will check for completion multiple times.
If there is a reliable way to test whether recvmsg had failed because of a write watch (and not somebody else) has invalidated the buffer and check that not only linux kernel but at least osx don't corrupt data in a subsequent call then I'd prefer that way as well. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #15 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- Created attachment 50161 --> https://bugs.winehq.org/attachment.cgi?id=50161 patch for ws2_32.recv Here is the patch that I'm using as a workaround, it works for the application I'm interested in. Obviously it's limited to ws2_32.recv and is not a general solution unlike Sebastian's approach. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #16 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- Hi Sebastian, after quite a bit of testing I prefer your approach. Unfortunately there is one thing that doesn't work with your patch. It's the asynchronous recv(). Actually that's not your fault, that's a general limitation of async callbacks implementation in Wine. wineserver sends a SIGUSR1 signal to a thread in order to execute an apc (server/thread.c,queue_apc()). USR1 signal handler (dlls/ntdll/signal_i386.c, usr1_handler()) shares the same code for suspending a thread with executing a scheduled apc, and it's executed on the signal stack. This means that during apc execution exceptions can't be handled, and in case of an exception the thread will be killed. An apc that is being executed calls dlls/ws2_32/socket.c, WS2_async_recv(), which in turn calls WS2_recv() and it triggers a write watch exception either by calling IsBadWritePtr() in your patch or memcpy() in mine hack => thread dies. In order to solve this either apc callback should avoid generating exceptions, or it should be executed on win32 stack instead, perhaps wine_call_on_stack is the way to achieve that. Avoiding generating exceptions seems unfeasible to me, VirtualQuery() doesn't provide a way to detect write watchable memory, and I don't see another way to determine why recvmsg has returned EFAULT. I'd appreciate your thoughts on this problem. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #17 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Dmitry Timoshkov from comment #16)
In order to solve this either apc callback should avoid generating exceptions, or it should be executed on win32 stack instead, perhaps wine_call_on_stack is the way to achieve that. Avoiding generating exceptions seems unfeasible to me, VirtualQuery() doesn't provide a way to detect write watchable memory, and I don't see another way to determine why recvmsg has returned EFAULT.
There is an obvious way to avoid generating exceptions and detecting a write watchable memory region - GetWriteWatch. But the apc callback can't avoid generating exceptions (by calling IsBadWritePtr) when recvmsg() returns EFAULT because that's the only way for ntdll to make the buffer writable before calling recvmsg() again. So the Sebastian's patch can't work for asynchronous winsock recv() calls without allowing exception handlers (at least inside of ntdll) work on a signal stack, but that's a serious architectural violation of current design of signal/SEH handling in wine. I'll attach a test application which demonstrates both the current problem with unexpected EFAULT return from asynchronous WSARecv(), and the receiver thread dying due to an exception on signal stack with Sebastian's patch. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #18 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- Created attachment 50350 --> https://bugs.winehq.org/attachment.cgi?id=50350 test application Here is the test application with pre-built .exe inside and sources. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #19 from Sebastian Lackner <sebastian(a)fds-team.de> --- (In reply to Dmitry Timoshkov from comment #16)
after quite a bit of testing I prefer your approach. Unfortunately there is one thing that doesn't work with your patch. It's the asynchronous recv(). Actually that's not your fault, that's a general limitation of async callbacks implementation in Wine.
Sorry, I was just too busy to answer during the last couple of days. Finally had time to take a closer look, and I have to admit that I didn't think about that while writing the patch. Pretty stupid design if you ask me, because exceptions can happen basically all the time while running async callbacks (programs messing around with page protections, ...). Best solution would be to introduce something like "wine kernel-mode" (not really executed in the kernel). Use a different stack, different SEH chain, disable all vectored exception handlers and so on. That would also allow to get rid of various other bugs, for example the issue that step-by-step execution shouldn't modify the stack below ESP. Nevertheless, such a big redesign is probably a bit too complicated. For Wine Staging it would be fine, but impossible to get upstream. I decided to fix it in my patchset by using a different method: Export new functions from ntdll for save memory access/permission check. My current series introduces the following changes: [1/3] Change virtual_uninterrupted_write_memory so that write-watches do not abort copying memory. Those are handled by the kernel on Windows, so they can be triggered without forwarding an exception to usermode. [2/3] Allow setting a NULL source/destination buffer for virtual_uninterrupted_[read|write]_memory, then the function will only check permissions (but still trigger write watches). Rename the functions and export them. [3/3] Similar to my previous patch, but use the newly exported functions. Please give it a try. Works well with your attached tests for me. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #20 from Sebastian Lackner <sebastian(a)fds-team.de> --- Oops, forgot to mention, new link is: https://github.com/wine-compholio/wine-staging/tree/master/patches/ws2_32-Wr... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #21 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #19)
Please give it a try. Works well with your attached tests for me.
Thank you Sebastian, new patch set works quite well, and indeed my test no longer shows a read error. Unfortunately the real application I have here still dies on an exception on signal thread after 15-20 minutes of work under pretty heavy load. I haven't figured out yet the reason, still investigating. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #22 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Dmitry Timoshkov from comment #21)
Please give it a try. Works well with your attached tests for me.
Thank you Sebastian, new patch set works quite well, and indeed my test no longer shows a read error. Unfortunately the real application I have here still dies on an exception on signal thread after 15-20 minutes of work under pretty heavy load. I haven't figured out yet the reason, still investigating.
s/signal thread/signal stack/ -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #23 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Dmitry Timoshkov from comment #21)
Please give it a try. Works well with your attached tests for me.
Thank you Sebastian, new patch set works quite well, and indeed my test no longer shows a read error. Unfortunately the real application I have here still dies on an exception on signal thread after 15-20 minutes of work under pretty heavy load. I haven't figured out yet the reason, still investigating.
Finally found time to investigate the problem. Sebastian's patch works perfectly for recv/WSARecv/recvfrom/WSARecvFrom, now the problem has appeared in another place: asynchronous callback of AcceptEx - WS2_async_accept. This callback writes into the application provided buffer all over the place, and I personally don't see a reasonable way to fix it without allowing SEH on a signal stack. A usual approach used in Wine code of checking the application provided buffer with IsBadWritePtr at the start of WS2_AcceptEx just postpones the crash and creates another race between the GC thread and winsock code. While investigating the problem with AcceptEx I have found another place where write watches cause a race and a lead to a failure to read normal disk file in a .net application I'm working on: it's ReadFile. While fixing ReadFile (actually NtReadFile) was trivial (I'll attach a patch) since it's not called on a signal stack, this shows that potentially any place of Wine code is vulnerable to the races caused by write watches. Just for the background: this .net application is a very simple client/server written using .net's System.Runtime.Remoting class. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #24 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- Created attachment 50691 --> https://bugs.winehq.org/attachment.cgi?id=50691 NtReadFile fix -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 Qian Hong <fracting(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fracting(a)gmail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #25 from Sebastian Lackner <sebastian(a)fds-team.de> --- Thanks for tracking this down. I don't think its easily possible to introduce proper exception handling on the signal stack. Exceptions introduced while wine is in "kernel mode" have to be handled completely different, and should never be forwarded to debuggers or other exception handlers, so a simple SEH2 exception handler is not sufficient. I don't think the problem with AcceptEx() is that hard to fix. If I didn't miss anything the patch I just added should be sufficient. Would you please test again? https://github.com/wine-compholio/wine-staging/tree/master/patches/ws2_32-Wr... The fix for NtReadFile is not complete unfortunately. Async handling of NtReadFile is handled in FILE_AsyncReadService(), and it should be changed there too (but you cannot use virtual_check_buffer_for_write because that is exception-based). In our Staging tree I have a patch to merge both functions, its part of the kernel32-NamedPipe patchset. I have rebased your changes on top of that. https://github.com/wine-compholio/wine-staging/tree/master/patches/kernel32-... https://github.com/wine-compholio/wine-staging/tree/master/patches/ntdll-Wri... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #26 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #25)
Thanks for tracking this down. I don't think its easily possible to introduce proper exception handling on the signal stack. Exceptions introduced while wine is in "kernel mode" have to be handled completely different, and should never be forwarded to debuggers or other exception handlers, so a simple SEH2 exception handler is not sufficient.
I don't think the problem with AcceptEx() is that hard to fix. If I didn't miss anything the patch I just added should be sufficient. Would you please test again?
https://github.com/wine-compholio/wine-staging/tree/master/patches/ws2_32- WriteWatches
With that patch applied in addition to previous series now the application crashes at (it's the match to the patched version) http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/ws2_32/socket.c#l2009 WS2_async_recv is called from WS2_async_accept_recv, which in turn is another async callback of AcceptEx. So, it's an improvement, the crash has moved to a later used code block. But it's really a pain to track all this down, although I have specially invented hacks to make my life a bit easier.
The fix for NtReadFile is not complete unfortunately. Async handling of NtReadFile is handled in FILE_AsyncReadService(), and it should be changed there too
I didn't need an async part (and couldn't test the fix) since the app here is reading from disk files, so I didn't bother with that. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #27 from Sebastian Lackner <sebastian(a)fds-team.de> --- Created attachment 50695 --> https://bugs.winehq.org/attachment.cgi?id=50695 ntdll: Try to handle write-watches while we're on the signal stack. Does the attached patch help (in addition to all the previous stuff, I hope it still applies ^^)? It should theoretically solve the problem, but I am not really glad yet about the way it is implemented. We can handle write-watches, but we still can't properly use __TRY/__EXCEPT blocks in APCs. Implementing all this properly would be much more complicated... :/ -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #28 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #27)
Created attachment 50695 [details] ntdll: Try to handle write-watches while we're on the signal stack.
Does the attached patch help (in addition to all the previous stuff, I hope it still applies ^^)? It should theoretically solve the problem, but I am not really glad yet about the way it is implemented. We can handle write-watches, but we still can't properly use __TRY/__EXCEPT blocks in APCs. Implementing all this properly would be much more complicated... :/
Patch applies but unfortunately I still get an exception at the same place http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/ws2_32/socket.c#l2009 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #29 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Dmitry Timoshkov from comment #28)
Patch applies but unfortunately I still get an exception at the same place http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/ws2_32/socket.c#l2009
It's caused by a typo in the patch: virtual_handle_fault() returns 0 (STATUS_SUCCESS) on success. After fixing that part the application works very well. Thanks! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #30 from Sebastian Lackner <sebastian(a)fds-team.de> --- (In reply to Dmitry Timoshkov from comment #29)
(In reply to Dmitry Timoshkov from comment #28)
Patch applies but unfortunately I still get an exception at the same place http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/ws2_32/socket.c#l2009
It's caused by a typo in the patch: virtual_handle_fault() returns 0 (STATUS_SUCCESS) on success. After fixing that part the application works very well. Thanks!
Oops, that was a stupid mistake. I even looked at the definition of the function... ;) Have added the patch at the end of the series. For those interested in testing the patches, you'll need: [1/3] https://github.com/wine-compholio/wine-staging/tree/master/patches/ws2_32-Wr... [2/3] https://github.com/wine-compholio/wine-staging/tree/master/patches/kernel32-... [3/3] https://github.com/wine-compholio/wine-staging/tree/master/patches/ntdll-Wri... Patch ws2_32-WriteWatches/0004 is not strictly required anymore because it should also be catched by the minimalistic exception handler in patch 0005. For patch 0003 it might be useful to keep the exception-free solution to avoid regressions in code which was working fine before (when the app really triggers an EFAULT). It can be replaced though when we have full exception handling support on the signal stack. For those who don't want the full Named_Pipe messagemode patchset, you can use https://bugs.winehq.org/attachment.cgi?id=50691, but keep in mind that it doesn't fix the async case. Happy testing! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #31 from Sebastian Lackner <sebastian(a)fds-team.de> --- One of the patches was accepted: http://source.winehq.org/git/wine.git/commit/6bec132c7ac6d21bb7da45b93d67bfa... Will submit a couple of more patches for the read() and write() syscalls soon. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #32 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #31)
One of the patches was accepted: http://source.winehq.org/git/wine.git/commit/ 6bec132c7ac6d21bb7da45b93d67bfad47ef8b58
Will submit a couple of more patches for the read() and write() syscalls soon.
Thanks Sebastian! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #33 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Sebastian Lackner from comment #31)
One of the patches was accepted: http://source.winehq.org/git/wine.git/commit/ 6bec132c7ac6d21bb7da45b93d67bfad47ef8b58
Will submit a couple of more patches for the read() and write() syscalls soon.
Hi Sebastian, are you still planning to send the patches? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 Andrew Eikum <aeikum(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aeikum(a)codeweavers.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 winetest(a)luukku.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |winetest(a)luukku.com --- Comment #34 from winetest(a)luukku.com --- Even the patch dont get merged for now can this bug be considered as staged? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 Dmitry Timoshkov <dmitry(a)baikal.ru> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |informacja.niejawna(a)gmail.c | |om --- Comment #35 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- *** Bug 41562 has been marked as a duplicate of this bug. *** -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 --- Comment #36 from Alexandre Julliard <julliard(a)winehq.org> --- Probably fixed now, please retest. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 Dmitry Timoshkov <dmitry(a)baikal.ru> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #37 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Alexandre Julliard from comment #36)
Probably fixed now, please retest.
I have no longer access to the large application that was originally mentioned, but the test case attached to this bug report works fine with wine-2.20-80-g2c51fc1bfc. Marking this bug as fixed. Thanks. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=37669 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #38 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 2.21. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org