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

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