http://bugs.winehq.org/show_bug.cgi?id=10368
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |focht@gmx.net
--- Comment #12 from Anastasius Focht focht@gmx.net 2008-08-09 05:51:36 --- Hello,
random pick of the day ;-) I downloaded the X3 trial version. James' analysis (comment #6) is correct so far.
Sub-installer custom action "CA_AcquireKey" fails due to MAIN_INSTALL_DLG_TITLE containing garbage. And yes, custom action "CA_Callit" which builds the action string contains some code which looks like subtile application bug.
In short: Differences in Wine and Windows heap block management let this app succeed in Windows - despite an application bug.
Following is the stack before a sequence of internal installer function calls which ultimately lead to the bug:
--- snip --- $+0 7ED445F8 ptr 009D0B28 $+4 1001C830 "%s SERIALNUMBER="%s"" $+8 009D0B28 "ACTIONTODO="AcquireKey" MAIN_INSTALL_DLG_TITLE="Setup Initialization"" $+C 7ED44604 "DR13WTX-9999998-YSP" .. --- snip ---
$+0 = pointer to destination buffer address (dest = heap allocated) $+4 = format string $+8 = first vararg parameter for format string call (heap allocated) $+C = second vararg parameter for format string call (stack allocated)
Notice how the destination buffer address is actually the same as first vararg parameter (part of problem).
Lets dissect the memory block of interest. I formatted all memory blocks in single dword format which makes them easier to read/comment (address | DWORD value | ASCII reprs. | comment).
--- snip #1 --- (wine heap block header) 009D0B10 00000058 X... ; ARENA_INUSE.size 009D0B14 02455355 USE. ; ARENA_INUSE.unused_bytes, ARENA_INUSE.magic (app internal buffer part) 009D0B18 10027450 Pt.. ; vtable pointer for some string manager 009D0B1C 00000045 E... ; length of currently used data 009D0B20 00000045 E... ; length of allocated data 009D0B24 00000001 .... ; reference count for this string object 009D0B28 49544341 ACTI ; begin of string buffer .. 009D0B68 6E6F6974 tion 009D0B6C 50410022 ".AP --- snip #1 ---
Following is the first free block which is of interest (there is an in-use block between both not of interest).
--- snip #2 --- .. 009D0BA0 00000221 !... ; ARENA_FREE.size 009D0BA4 45455246 FREE ; ARENA_FREE.magic 009D0BA8 009D00F8 ø.. ; ARENA_FREE.list_entry.FLink 009D0BAC 009D00E8 è.. ; ARENA_FREE.list_entry.BLink 009D0BB0 6F646E69 indo ... --- snip #2 ---
Within the buggy sequence, a reallocation happens, hence the memory block layout is as follows:
First memory block (which was previously allocated):
--- snip #3 --- 009D0B10 00000051 Q... ; ARENA_FREE.size 009D0B14 45455246 FREE ; ARENA_FREE.magic 009D0B18 009D00B8 ... ; ARENA_FREE.list_entry.FLink 009D0B1C 009D00A8 ... ; ARENA_FREE.list_entry.BLink
009D0B20 00000045 E... 009D0B24 00000001 .... 009D0B28 49544341 ACTI .. 009D0B68 6E6F6974 tion 009D0B6C 009D0B10 ... ; --- snip #3 ---
Directly after (re)allocation: Previously free, now newly allocated block to hold the result data.
--- snip #4 --- 009D0BA0 000000A0 .... 009D0BA4 05455355 USE. 009D0BA8 10027450 Pt.. 009D0BAC 00000045 E... 009D0BB0 0000008A Š... 009D0BB4 00000001 .... 009D0BB8 49544341 ACTI 009D0BBC 4F544E4F ONTO 009D0BC0 223D4F44 DO=" 009D0BC4 75716341 Acqu 009D0BC8 4B657269 ireK 009D0BCC 20227965 ey" 009D0BD0 4E49414D MAIN 009D0BD4 534E495F _INS 009D0BD8 4C4C4154 TALL 009D0BDC 474C445F _DLG 009D0BE0 5449545F _TIT 009D0BE4 223D454C LE=" 009D0BE8 75746553 Setu 009D0BEC 6E492070 p In 009D0BF0 61697469 itia 009D0BF4 617A696C liza 009D0BF8 6E6F6974 tion 009D0BFC 69570022 ".Wi ... --- snip #4 ---
And the same block after bug sequence:
--- snip #5 --- 009D0BA0 000000A0 .... 009D0BA4 05455355 USE. 009D0BA8 10027450 Pt.. 009D0BAC 00000045 E... 009D0BB0 0000008A Š... 009D0BB4 00000001 .... 009D0BB8 49544341 ACTI .. 009D0BF0 61697469 itia 009D0BF4 617A696C liza 009D0BF8 6E6F6974 tion 009D0BFC 209D0B10 .. 009D0C00 49524553 SERI 009D0C04 554E4C41 ALNU 009D0C08 5245424D MBER 009D0C0C 5244223D ="DR 009D0C10 54573331 13WT 009D0C14 39392D58 X-99 009D0C18 39393939 9999 009D0C1C 53592D38 8-YS 009D0C20 00002250 P".. .. --- snip #5 ---
Ok, that's a lot of heap block dumps ... let me explain what happens.
I'll provide a pseudo code sequence which I think resembles the bug behaviour. It might not be 100% accurate, just to illustrate the problem.
Consider the following snippet:
--- pseudo code --- void format_string_memfn( const char* formatstring, ...) { va_list ap; va_start( ap, formatstring);
int required_length = get_required_length_for_format_operation( formatstring, ap); if( internal_string_buffer.length < required_length) reallocate_internal_buffer( required_length);
vsnprintf( internal_string_buffer, required_length, formatstring, ap); va_end(ap); }
// instanciate string class object some_string_class str; .. // do some formatstring ops on string object // str's internal buffer now contains "ACTIONTODO="AcquireKey" MAIN_INSTALL_DLG_TITLE="Setup Initialization"
// I am evil str.format_string_memfn( "%s SERIALNUMBER="%s"", (const char*) str, "DR13WTX-9999998-YSP");
--- pseudo code ---
The internal format function checks if the expected result buffer size of format operation is larger than internal storage. If so, it reallocates the internal buffer (while preserving the content). snippet #3 = previously allocated heap block, now marked free snippet #4 = new buffer
The string format operation then copies the data into newly allocated buffer.
Now the problem: noticed the first vararg parameter? It's actually a "raw" pointer to the internal representation of the string buffer - at the time before the function call. Most string classes provide such "operator const char * () const" to get a C string representation of underlying object. This means that even after internal reallocation to new heap block, the old storage is still referenced by this parameter/pointer (va_list).
When the string copying for the first vararg param occurs, the old (now freed) block contents are copied. If you look closely at snippet #3 last DWORD you see "009D0B10". It's actually a back pointer, set by Wine's heap manager when the block has been marked as free. A part of string data has been overwritten. The string copy operation copies until it encounters null terminator, hence this back pointer data is taken as string data (see "209D0B10" data in snippet #5).
Because Wine's heap manager is actually touching the data area by writing the back pointer value to last 4 bytes when it marks the block as "free", the data integrity is not preserved. This behaviour is completely valid because application shall not touch the old block anymore. I can only guess that Windows heap manager preserves the old storage until next heap calls/consolidation which lets this application succeed despite this nasty bug. If heap managers would clear heap blocks by default I guess many applications will not work anymore due to such brain damaged code ;-)
I won't provide a patch here. Wine's heap manager is something Julliard probably wants to fix himself.
Just a hint: if you keep the back pointer for free blocks at this place (directly before the next arena header), provide sizeof(ARENA_FREE*) more storage, to not let the back pointer overwrite data parts.
I already tested it - works ;-) If this bug is fixed, you'll run into bug 2547 I'll have a look into that too ...
Regards