Dimitrie O. Paun wrote:
On December 22, 2002 02:50 pm, Duane Clark wrote:

That area is then freed. When freed, COMCTL32_Free writes prev and next
entries into himl.

Where? COMCTL32_Free() simply calls HeapFree():

BOOL WINAPI COMCTL32_Free (LPVOID lpMem)
{
    TRACE("(%p)\n", lpMem);
    return HeapFree (COMCTL32_hHeap, 0, lpMem);
}
HeapFree then calls RtlFreeHeap, which calls HEAP_MakeInUseBlockFree, and that writes the prev and next entries. It appears that Wine keeps a linked list of free blocks on the heap, with the link entries in the first and second words of the block.

At the end of ImageList_Destroy, there is a call to ZeroMemory, which
obliterates the prev and next pointers which had been written there.
Then another COMCTL32_Free call detects the error. At least I assume it
is an error.

Does this work any better? It shouldn't make any difference, but ...
Nope.

There is something else funny here. I can't see why zeroing out an
area we're just about to free is problematic.
The first time, when it was a valid block, zeroing out the area before freeing was fine. When the block was freed, HEAP_MakeInUseBlockFree marked it as free and then wrote the link list entries into it, overwriting the zeroes that had just been put there.

It would appear that at a minimum, a test needs to be made in ImageList_Destroy to determine whether the heap corresponding to the imagelist is valid.

Even better, there is already a magic field in the _IMAGELIST structure, which does not appear to be used. Perhaps it should have "used" and "free" magic values, similar to what is used by the heap.

It really would not matter if the "free" value is subsequently overwritten, either by the heap functions or something else to which the block was allocated, since it would not be likely to get the "used" magic written to it; unless of course it was allocated to another imagelist.




Reply via email to