[Bug 16047] New: Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

Summary: Wine and WSASendTo GARENA crash access violation
           Product: Wine
           Version: 1.1.8
          Platform: PC
               URL: http://garena.com
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: major
          Priority: P1
         Component: winsock
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

WINE 1.1.8
Garena client is crash war3.exe in function WSASendTo in war3hook.dll (Garena
hook)
Don't check if  parameter is NULL.
Winedbg trace in attachment.

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

Jeff Zaroyko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
           Severity|major                       |normal
           Priority|P1                          |P2

--- Comment #1 from Jeff Zaroyko   2008-11-14 04:22:29 ---
not major.  please attach the backtrace

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #2 from neskaju_bI   2008-11-14 04:24:05 ---
Created an attachment (id=17273)
 --> (http://bugs.winehq.org/attachment.cgi?id=17273)
trace

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

neskaju_bI  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |tasklist

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

Jeff Zaroyko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|tasklist                    |

--- Comment #3 from Jeff Zaroyko   2008-11-14 04:29:32 ---
can you attach the regular terminal output from the application rather than
your winedbg session?

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #4 from Jeff Zaroyko   2008-11-14 04:47:24 ---
a +winsock trace would be useful also

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #5 from neskaju_bI   2008-11-14 07:10:10 ---
Created an attachment (id=17275)
 --> (http://bugs.winehq.org/attachment.cgi?id=17275)
console output from command wine Garena.exe

I do not know as to receive output from process war3.exe since it starts itself
Garena

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #6 from neskaju_bI   2008-11-14 07:12:29 ---
Has forgotten to mention process war3.exe it was started only from 4th time!

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

neskaju_bI  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #17275|0                           |1
        is obsolete|                            |

--- Comment #7 from neskaju_bI   2008-11-14 08:41:49 ---
Created an attachment (id=17276)
 --> (http://bugs.winehq.org/attachment.cgi?id=17276)
WINEDEBUG=+ws2,+seh,+tid,+process

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

neskaju_bI  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #17276|0                           |1
        is obsolete|                            |

--- Comment #8 from neskaju_bI   2008-11-14 08:51:25 ---
Created an attachment (id=17277)
 --> (http://bugs.winehq.org/attachment.cgi?id=17277)
REUPLOAD WINEDEBUG=+ws2,+seh,+tid,+process

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #9 from neskaju_bI   2008-11-14 09:49:15 ---
Created an attachment (id=17278)
 --> (http://bugs.winehq.org/attachment.cgi?id=17278)
WINEDEBUG=+winsock,+seh,+tid,+process

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

neskaju_bI  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #17277|0                           |1
        is obsolete|                            |

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #10 from Damjan Jovanovic   2008-11-15 04:11:05 ---
Created an attachment (id=17298)
 --> (http://bugs.winehq.org/attachment.cgi?id=17298)
Verify OVERLAPPED hEvent before use

Hello

For your 2 traces, it looks like the crashes are in completely different
places, but please try this patch.

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

Damjan Jovanovic  changed:

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

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #11 from neskaju_bI   2008-11-15 05:24:36 ---
The patch has not helped, an error in the same place

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

Lukáš Krejza  changed:

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

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

neskaju_bI  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1

--- Comment #12 from neskaju_bI   2008-11-18 01:04:00 ---
*** This bug has been confirmed by popular vote. ***

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #13 from Lukáš Krejza   2008-12-22 19:46:57 ---
I think we can mark this as 1.2 milestone... Since garena has 32votes which
makes it 40th most wanted app... :-)

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #14 from Lukáš Krejza   2009-03-08 13:54:12 ---
Really no resolution possible? :-(

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

Matej Spindler  changed:

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

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

Mike Kaplinskiy  changed:

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

--- Comment #15 from Mike Kaplinskiy   2009-09-01 23:34:58 ---
I don't know if it's related but the warcraft binary fails to start:

0044:trace:seh:raise_exception code=c0000005 flags=0 addr=0x7b85a60b
0044:trace:seh:raise_exception  info[0]=00000001
0044:trace:seh:raise_exception  info[1]=00000008
0044:trace:seh:raise_exception  eax=00000000 ebx=7b8b2820 ecx=00000001
edx=00000000 esi=601419a0 edi=00000001
0044:trace:seh:raise_exception  ebp=0033fad8 esp=0033fa80 cs=0073 ds=007b
es=007b fs=0033 gs=003b flags=00210246
0044:trace:seh:call_stack_handlers calling handler at 0x7bc46bd0 code=c0000005
flags=0
0044:trace:seh:__regs_RtlUnwind code=c0000005 flags=2
0044:trace:seh:__regs_RtlUnwind calling handler at 0x7bc3a110 code=c0000005
flags=2
0044:trace:seh:__regs_RtlUnwind handler at 0x7bc3a110 returned 1
0044:err:module:attach_process_dlls "gdi32.dll" failed to initialize, aborting
0044:err:module:LdrInitializeThunk Main exe initialization for L"C:\Program
Files\Warcraft III\war3.exe" failed, status c0000005

This is before the last exception, which I'm not even sure is in wine code. How
clean is your prefix?

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

Jeff Zaroyko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Alias|WSASendTo_crash             |

--- Comment #16 from Jeff Zaroyko   2009-10-15 23:33:47 ---
(In reply to comment #15)
> I don't know if it's related but the warcraft binary fails to start:
>
> This is before the last exception, which I'm not even sure is in wine code. How
> clean is your prefix?

Similar thing seems to happen in bug 15518

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

Pigeon  changed:

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

--- Comment #17 from Pigeon   2010-07-14 08:56:31 ---
I've been testing this recently, using wine 1.2 + acceptex patch or
git://repo.or.cz/wine/hacks.git

Both still have the same problem when you actually join a game.

My understand is:

war3 calls WS_send or WSASend
WS_send/WSASend calls WSASendTo with a NULL lpTo
war3hook.dll catches WSASendTo, and not checking the NULL lpTo.

I have tried sending a dummy lpTo and it seems to fix the NULL dereference, but
it fails to join a game. I guess I'm not sure about how I should setup lpTo
(addr, port, etc).

Another interesting thing is, if I have WINEDEBUG=+relay, the problem somehow
goes away, I can join and play a full game.

P.S. Since wine 1.1.26, Garena GUI won't come up unless you kill the
winedevice.exe GarenaPEngine process.

[Bug 16047] Wine and WSASendTo GARENA crash access violation

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #18 from Pigeon   2010-07-15 07:51:31 ---

I have posted a new comment at
http://appdb.winehq.org/objectManager.php?sClass=version&iId=19336
which detailed what I have done to get Garena + Warcraft3 going.
Of course it would be good if we could somehow fix the crash when running
without WINEDEBUG=+relay.

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Wine and WSASendTo GARENA   |Garena client crashes in
                   |crash access violation      |WSASendTo

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #19 from Mike Kaplinskiy   2010-07-15 20:14:32 ---
Since you're willing to work on this, see if the following works. Create a
helper that takes the same parameters as WSASendTo/WSARecvFrom. Then change all
calls to WSASendTo/WSARecvFrom in our ws2_32 code to use the helper instead.
Warcraft probably uses several types of send calls within it and garena doesn't
expect out of order calls from us.

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #20 from Pigeon   2010-07-16 08:26:37 ---
(In reply to comment #19)
> Since you're willing to work on this, see if the following works. Create a
> helper that takes the same parameters as WSASendTo/WSARecvFrom. Then change all
> calls to WSASendTo/WSARecvFrom in our ws2_32 code to use the helper instead.
> Warcraft probably uses several types of send calls within it and garena doesn't
> expect out of order calls from us.

Thanks for the pointer.

However I'm not familiar with the ws2 apis and and not sure how they work.
Did you mean the wine implementation of WSASendTo and WSARecvFrom are done
asynchronously, which might be causing problem?

Also, did you mean having one helper function that handles both WSASendTo and
WSARecvFrom?

Thanks again.

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #21 from Mike Kaplinskiy   2010-07-16 14:25:03 ---
(In reply to comment #20)
> Thanks for the pointer.
>
> However I'm not familiar with the ws2 apis and and not sure how they work.
> Did you mean the wine implementation of WSASendTo and WSARecvFrom are done
> asynchronously, which might be causing problem?
>
> Also, did you mean having one helper function that handles both WSASendTo and
> WSARecvFrom?
>
> Thanks again.

You don't need to be familiar with the API's, just C. You should create (just
rename works) two helpers (one for each) for WSASendTo and WSARecvFrom. Then
replace all calls to WSASendTo/WSARecvFrom within ws2_32 to call the helpers
instead (you can do 90% of that with find/replace). Don't forget to rewrite the
actual WSARecvFrom/WSASendTo and have them call the helper as well.

The problem is that Warcraft uses different socket layers in different places.
In some places it might call send/WSASend and recv/WSARecv whereas in others it
uses WSASendTo and WSARecvFrom. Garena looks to hook only
WSASendTo/WSARecvFrom. This is because it probably only wants to intercept
packets for connecting to a game and not whatever else Warcraft does.

Wine implements send/WSASend and recv/WSARecv by calling WSASendTo/WSARecvFrom
which ends up going to Garena code and confusing Garena since it only expects
calls from Warcraft. So if we stop calling WSASendTo/WSARecvFrom from within
wine code (by calling the helper instead) Garena won't receive any calls and
should be happy. I don't know if it will fix all the problems, but I think
that's what is happening.

I'd write the patch for you, but I'm not around a git tree. See if you can do
it yourself with a bit of help from #winehq

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #22 from Pigeon   2010-07-16 20:30:40 ---
That's it!

I have copied the two functions, rename them (I just added _helper suffix), and
replaced all calls to them within socket.c to call to helper. Now Garena + war3
works!

Now the question is if such a patch would be valid and worthy for all wine
users.

Also, having two functions with different names but exactly the same definition
look kind of silly. Any better way of doing this? I thought of using a #define
to define the two functions but that is ugly too.

Thanks again for your help!

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #23 from Mike Kaplinskiy   2010-07-16 23:27:00 ---
A patch with that idea would be fine. And no, you can't do a define.
WSASendTo/WSARecvFrom are exported so they must be declared. Don't worry about
the "duplication". Just make sure that it looks clean and compiles without
warnings. Then you can send it to wine-patches. You'll either get feedback or
it will be committed.

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #24 from Pigeon   2010-07-17 07:14:41 ---
Created an attachment (id=29646)
 --> (http://bugs.winehq.org/attachment.cgi?id=29646)
ws2_32 not calling WSASendTo/WSARecvFrom internally

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #25 from Pigeon   2010-07-17 07:17:24 ---
The previous patch made sure WSASendTo and WSARecvFrom are not called
internally from ws2_32 (socket.c)
Seems to have avoided the crash in Garena's War3hook.dll

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #26 from Dmitry Timoshkov   2010-07-17 22:40:34 ---
You may try to make the patch less aggressive, and change only Ex versions
of the API, leaving not Ex ones just call the Ex versions and not the helpers.

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #27 from Mike Kaplinskiy   2010-07-17 23:41:58 ---
(In reply to comment #26)
> You may try to make the patch less aggressive, and change only Ex versions
> of the API, leaving not Ex ones just call the Ex versions and not the helpers.

I'm not really sure what you're suggesting, considering there are no Ex
functions involved. The problem is that WSASendTo is being hooked by garena so
we shouldn't call it within our code since windows doesn't do it like that (and
writing a test for that is IMHO excessive). No one is defining any new exported
functions.

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #28 from Dmitry Timoshkov   2010-07-18 00:22:56 ---
(In reply to comment #27)
> I'm not really sure what you're suggesting, considering there are no Ex
> functions involved. The problem is that WSASendTo is being hooked by garena so
> we shouldn't call it within our code since windows doesn't do it like that (and
> writing a test for that is IMHO excessive). No one is defining any new exported
> functions.

I'm suggesting to test if that's enough to wrap only things like WS_recvfrom,
WSASendTo and not WS_recv, WSASend, WS_sendto. I.e. try to find minimal set
of required wrappers.

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

--- Comment #29 from Mike Kaplinskiy   2010-07-18 01:03:54 ---
(In reply to comment #28)
> I'm suggesting to test if that's enough to wrap only things like WS_recvfrom,
> WSASendTo and not WS_recv, WSASend, WS_sendto. I.e. try to find minimal set
> of required wrappers.

Ah, I see now. Personally I'd prefer the approach of replacing all of them (for
consistency), but you also have a point. Whatever Alexandre decides will go I
suppose :)

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

Austin English  changed:

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

--- Comment #30 from Austin English   2010-07-19 12:07:59 ---
Fixed by
http://source.winehq.org/git/wine.git/?a=commitdiff;h=8b7ec05bd20359662f4feb...

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

Alexandre Julliard  changed:

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

--- Comment #31 from Alexandre Julliard   2010-07-30 12:56:18 ---
Closing bugs fixed in 1.3.0.

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |1.2.x

[Bug 16047] Garena client crashes in WSASendTo

http://bugs.winehq.org/show_bug.cgi?id=16047

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|1.2.x                       |---

--- Comment #32 from Alexandre Julliard  2010-10-08 10:40:32 CDT ---
Removing 1.2.x milestone from bugs fixed in 1.2.1.