[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: [e-mail]
          Reporter: [e-mail]
      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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Dmitry Timoshkov  ---
AF might be interested :)

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

Sebastian Lackner  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #2 from Sebastian Lackner  ---
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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #3 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #4 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #5 from Sebastian Lackner  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #6 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #7 from Sebastian Lackner  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #8 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #9 from Bruno Jesus  ---
Is it ok to set the component as winsock? Dmitry can you share your patch to
solve this?

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #10 from Sebastian Lackner  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #11 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #12 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #13 from Sebastian Lackner  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #14 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #15 from Dmitry Timoshkov  ---
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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #16 from Dmitry Timoshkov  ---
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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #17 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #18 from Dmitry Timoshkov  ---
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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #19 from Sebastian Lackner  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #20 from Sebastian Lackner  ---
Oops, forgot to mention, new link is:
https://github.com/wine-compholio/wine-staging/tree/master/patches/ws2_32-Wr...

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #21 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #22 from Dmitry Timoshkov  ---
(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/

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #23 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #24 from Dmitry Timoshkov  ---
Created attachment 50691
  --> https://bugs.winehq.org/attachment.cgi?id=50691
NtReadFile fix

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

Qian Hong  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #25 from Sebastian Lackner  ---
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...

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #26 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #27 from Sebastian Lackner  ---
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... :/

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #28 from Dmitry Timoshkov  ---
(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

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #29 from Dmitry Timoshkov  ---
(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!

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #30 from Sebastian Lackner  ---
(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!

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #31 from Sebastian Lackner  ---
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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #32 from Dmitry Timoshkov  ---
(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!

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #33 from Dmitry Timoshkov  ---
(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?

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

Andrew Eikum  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #34 from [e-mail] ---
Even the patch dont get merged for now can this bug be considered as staged?

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |informacja.niejawna@gmail.c
                   |                            |om

--- Comment #35 from Dmitry Timoshkov  ---
*** Bug 41562 has been marked as a duplicate of this bug. ***

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

--- Comment #36 from Alexandre Julliard  ---
Probably fixed now, please retest.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #37 from Dmitry Timoshkov  ---
(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.

[Bug 37669] Resetting a write watch can cause memory access violation in kernel

https://bugs.winehq.org/show_bug.cgi?id=37669

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #38 from Alexandre Julliard  ---
Closing bugs fixed in 2.21.