https://bugs.winehq.org/show_bug.cgi?id=36387
Bug ID: 36387 Summary: Multiple Microsoft installers crash or hang with heap corruption (XmlLite, XPSEP, IE7) Product: Wine Version: 1.7.18 Hardware: x86 OS: Linux Status: NEW Severity: normal Priority: P2 Component: setupapi Assignee: wine-bugs@winehq.org Reporter: focht@gmx.net
Hello folks,
continuation of bug 26016
I did more testing, trying to extract a pattern out of the crashes. Unfortunately there is no clear pattern.
There was one crash log which gave me another idea though.
--- snip --- $ WINEDEBUG=+tid,+seh,+relay,+heap wine ./update.exe -q >>log.txt 2>&1 ... 0035:Call setupapi.SetupInitDefaultQueueCallbackEx(00000000,ffffffff,00000000,00000000,00000000) ret=01053eac 0035:Call ntdll.RtlAllocateHeap(00110000,00000000,0000000c) ret=7e0c34d9 0035:trace:heap:RtlAllocateHeap (0x110000,70000062,0000000c): returning 0x1b70c0 0035:Ret ntdll.RtlAllocateHeap() retval=001b70c0 ret=7e0c34d9 0035:Ret setupapi.SetupInitDefaultQueueCallbackEx() retval=001b70c0 ret=01053eac ... 0035:Call setupapi.SetupCommitFileQueueA(00000000,001f3908,7e09e35c,001b70c0) ret=01053f55 0035:Ret setupapi.SetupCommitFileQueueA() retval=00000001 ret=01053f55 ... 0035:Call setupapi.SetupCommitFileQueueA(00000000,0011c5a0,0103be4b,0034c1b8) ret=01055f68 0035:Call setupapi.SetupDefaultQueueCallbackA(001b70c0,00000001,00000000,00000000) ret=0103bf44 0035:Ret setupapi.SetupDefaultQueueCallbackA() retval=00000001 ret=0103bf44 0035:Call ntdll.RtlFreeHeap(00110000,00000000,00000000) ret=7e0bf759 0035:Ret ntdll.RtlFreeHeap() retval=00000001 ret=7e0bf759 0035:Call ntdll.RtlAllocateHeap(00110000,00000000,00000080) ret=7e0bf782 0035:err:heap:HEAP_ValidateInUseArena Heap 0x110000: free block 0x1b70e0 overwritten at 0x1b70f0 by ffffffff ... --- snip ---
There was a damaged heap block located near the context structure for the default queue callback.
MSDN: http://msdn.microsoft.com/en-us/library/aa376956%28v=vs.85%29.aspx
--- quote --- The SetupInitDefaultQueueCallback function builds the context structure that is used by the default queue callback routine. It returns a void pointer to that structure. This structure is essential for the default callback routine's operation and must be passed to the callback routine. You do can this either by specifying the void pointer as the context in a call to SetupCommitFileQueue, or by specifying the void pointer as the context parameter when calling SetupDefaultQueueCallback from a custom callback routine. This context structure must not be altered or referenced by the setup application. --- quote ---
What if these guys writing the update installers, distributing their own copy of setupapi (updspapi) give a damn about MSDN and treat the context callback structure not as opaque, possibly peek/modify structure members?
The context structured started at 0x001b70c0 .. the damaged part was at 0x1b70f0. Unfortunately, I could not replicate the corruption pattern at this offset with more tests.
I gave the callback context structure a larger block - 0x40 bytes to be safe. The block was allocated with HEAP_ZERO_MEMORY to make sure that even if the installer code peeks into structure it doesn't see garbage. The current structure layout (member offsets) is most likely different from their layout but that doesn't seem to play a role here.
With that change in place the crashes were gone. I got a random heap crit sec hang out of several hundred tries though (maybe unrelated?).
Austin, can you pad the structure here up to 0x40 bytes:
http://source.winehq.org/git/wine.git/blob/4d796458d0ed517d45adc57a1aedaf1c3...
--- snip --- 39 /* context structure for the default queue callback */ 40 struct default_callback_context 41 { 42 HWND owner; 43 HWND progress; 44 UINT message; 45 }; --- snip ---
zero init alloc (HEAP_ZERO_MEMORY) here: http://source.winehq.org/git/wine.git/blob/4d796458d0ed517d45adc57a1aedaf1c3...
--- snip --- 1480 PVOID WINAPI SetupInitDefaultQueueCallbackEx( HWND owner, HWND progress, UINT msg, 1481 DWORD reserved1, PVOID reserved2 ) 1482 { 1483 struct default_callback_context *context; 1484 1485 if ((context = HeapAlloc( GetProcessHeap(), 0, sizeof(*context) ))) 1486 { 1487 context->owner = owner; 1488 context->progress = progress; 1489 context->message = msg; 1490 } 1491 return context; 1492 } --- snip ---
and retest? Thanks.
Regards
https://bugs.winehq.org/show_bug.cgi?id=36387
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, Installer URL| |http://download.microsoft.c | |om/download/f/9/6/f964059a- | |3747-4ed8-9326-ba1e639031b1 | |/WindowsXP-KB915865-v11-x86 | |-ENU.exe
http://bugs.winehq.org/show_bug.cgi?id=36387
--- Comment #1 from Nikolay Sivov bunglehead@gmail.com --- Created attachment 48439 --> http://bugs.winehq.org/attachment.cgi?id=48439 patch
To be absolutely safe you needed 0x50 bytes :). Please try this one. I dumped returned buffer, some fields are obvious, some not. One DWORD gets different value on each run, no idea what it means (could be a temporary window handle or something else).
https://bugs.winehq.org/show_bug.cgi?id=36387
--- Comment #2 from Austin English austinenglish@gmail.com --- (In reply to Nikolay Sivov from comment #1)
Created attachment 48439 [details] patch
To be absolutely safe you needed 0x50 bytes :). Please try this one. I dumped returned buffer, some fields are obvious, some not. One DWORD gets different value on each run, no idea what it means (could be a temporary window handle or something else).
I got a critical section timeout on the 463rd try of xmllite: + grep err:ntdll:RtlpWaitForCriticalSection xml-git-normal/xmllite-loop-463.txt err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main process heap section" wait timed out in thread 0041, blocked by 0042, retrying (60 sec) err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main process heap section" wait timed out in thread 0041, blocked by 0042, retrying (60 sec) err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main process heap section" wait timed out in thread 0041, blocked by 0042, retrying (60 sec) + exit 1
real 169m19.493s user 13m38.288s sys 4m33.669s
https://bugs.winehq.org/show_bug.cgi?id=36387
--- Comment #3 from Austin English austinenglish@gmail.com --- And in ie7 after 17 tries: + grep err:ntdll:RtlpWaitForCriticalSection ie7-git-heap/ie7-loop-17.txt err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main process heap section" wait timed out in thread 003b, blocked by 002c, retrying (60 sec) err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main process heap section" wait timed out in thread 003b, blocked by 002c, retrying (60 sec) err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main process heap section" wait timed out in thread 003b, blocked by 002c, retrying (60 sec) + exit 1
https://bugs.winehq.org/show_bug.cgi?id=36387
--- Comment #4 from Anastasius Focht focht@gmx.net --- Hello Austin,
is there any difference to tests without callback context patch then? What about XPSep installer, is that still an issue there too?
Regards
https://bugs.winehq.org/show_bug.cgi?id=36387
--- Comment #5 from Austin English austinenglish@gmail.com --- (In reply to Austin English from comment #3)
And in ie7 after 17 tries:
- grep err:ntdll:RtlpWaitForCriticalSection ie7-git-heap/ie7-loop-17.txt
err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main process heap section" wait timed out in thread 003b, blocked by 002c, retrying (60 sec) err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main process heap section" wait timed out in thread 003b, blocked by 002c, retrying (60 sec) err:ntdll:RtlpWaitForCriticalSection section 0x110060 "heap.c: main process heap section" wait timed out in thread 003b, blocked by 002c, retrying (60 sec)
- exit 1
If I ignore heap errors, then IE7 installs 500 times in a row, both with and without warn+heap.
https://bugs.winehq.org/show_bug.cgi?id=36387
--- Comment #6 from Austin English austinenglish@gmail.com --- (In reply to Anastasius Focht from comment #4)
Hello Austin,
is there any difference to tests without callback context patch then? What about XPSep installer, is that still an issue there too?
Regards
Without the patch, they still fail for me, see https://bugs.winehq.org/show_bug.cgi?id=26016#c40
Haven't tested XPSep, I'll give it a try when I get some free time next week.
https://bugs.winehq.org/show_bug.cgi?id=36387
--- Comment #7 from Anastasius Focht focht@gmx.net --- Hello Austin,
thanks for testing.
--- quote --- If I ignore heap errors, then IE7 installs 500 times in a row, both with and without warn+heap. ... Without the patch, they still fail for me --- quote ---
So it's definitely an improvement. Even if the structure is internal and opaque to clients it doesn't harm if Nikolay sends a patch in, maybe accompanied with a small test case that calls SetupInitDefaultQueueCallbackEx() and checks the known member offsets for expected data values (magic, owner, progress, msg)
Regards
http://bugs.winehq.org/show_bug.cgi?id=36387
--- Comment #8 from Nikolay Sivov bunglehead@gmail.com --- Patch is committed as dcab5fe61bed87b291901ceff686894a64871d98.
http://bugs.winehq.org/show_bug.cgi?id=36387
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |dcab5fe61bed87b291901ceff68 | |6894a64871d98 Status|NEW |RESOLVED Resolution|--- |FIXED
--- Comment #9 from Anastasius Focht focht@gmx.net --- Hello Nikolay,
thanks. The likelihood of heap corruption should be now much lower.
http://source.winehq.org/git/wine.git/commitdiff/dcab5fe61bed87b291901ceff68...
@Austin
If there is still a significant frequency for one installer (normal use, without +heap), please create a new bug. I might have another look if I get bored one day :)
Regards
http://bugs.winehq.org/show_bug.cgi?id=36387
--- Comment #10 from Austin English austinenglish@gmail.com --- (In reply to Anastasius Focht from comment #9)
Hello Nikolay,
thanks. The likelihood of heap corruption should be now much lower.
http://source.winehq.org/git/wine.git/commitdiff/ dcab5fe61bed87b291901ceff686894a64871d98
@Austin
If there is still a significant frequency for one installer (normal use, without +heap), please create a new bug. I might have another look if I get bored one day :)
Regards
Thanks, I've marked it as fixed in winetricks as well: https://code.google.com/p/winetricks/source/detail?r=1188
https://bugs.winehq.org/show_bug.cgi?id=36387
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #11 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 1.7.19.
https://bugs.winehq.org/show_bug.cgi?id=36387
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- URL|http://download.microsoft.c |https://web.archive.org/web |om/download/f/9/6/f964059a- |/20120525055657/http://down |3747-4ed8-9326-ba1e639031b1 |load.microsoft.com/download |/WindowsXP-KB915865-v11-x86 |/2/5/2/2526f55d-32bc-410f-b |-ENU.exe |e18-164ba67ae07d/XPSEP%20XP | |%20and%20Server%202003%2032 | |%20bit.msi