[Bug 36387] New: Multiple Microsoft installers crash or hang with heap corruption (XmlLite, XPSEP, IE7)
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(a)winehq.org Reporter: focht(a)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 -- 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=36387 Anastasius Focht <focht(a)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 -- 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.
http://bugs.winehq.org/show_bug.cgi?id=36387 --- Comment #1 from Nikolay Sivov <bunglehead(a)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). -- 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=36387 --- Comment #2 from Austin English <austinenglish(a)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 -- 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=36387 --- Comment #3 from Austin English <austinenglish(a)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 -- 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=36387 --- Comment #4 from Anastasius Focht <focht(a)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 -- 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=36387 --- Comment #5 from Austin English <austinenglish(a)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. -- 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=36387 --- Comment #6 from Austin English <austinenglish(a)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. -- 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=36387 --- Comment #7 from Anastasius Focht <focht(a)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 -- 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.
http://bugs.winehq.org/show_bug.cgi?id=36387 --- Comment #8 from Nikolay Sivov <bunglehead(a)gmail.com> --- Patch is committed as dcab5fe61bed87b291901ceff686894a64871d98. -- 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.
http://bugs.winehq.org/show_bug.cgi?id=36387 Anastasius Focht <focht(a)gmx.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |dcab5fe61bed87b291901ceff68 | |6894a64871d98 Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #9 from Anastasius Focht <focht(a)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 -- 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.
http://bugs.winehq.org/show_bug.cgi?id=36387 --- Comment #10 from Austin English <austinenglish(a)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 -- 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=36387 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #11 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 1.7.19. -- 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=36387 Anastasius Focht <focht(a)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 -- 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 (2)
-
wine-bugs＠winehq.org -
WineHQ Bugzilla