[PATCH 1/5] kernel32/tests: Add some HEAP_NO_SERIALIZE flag / LFH locking tests.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/kernel32/tests/heap.c | 202 ++++++++++++++++++++++++++++++++++++- 1 file changed, 201 insertions(+), 1 deletion(-) diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index fded1c69f1e..28cbee6ea83 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -88,18 +88,76 @@ struct heap }; +struct heap_thread_params +{ + HANDLE ready_event; + HANDLE start_event; + BOOL done; + + HANDLE heap; + DWORD flags; + BOOL lock; +}; + +DWORD WINAPI heap_thread_proc( void *arg ) +{ + struct heap_thread_params *params = arg; + void *ptr; + DWORD res; + BOOL ret; + + SetEvent( params->ready_event ); + + while (!(res = WaitForSingleObject( params->start_event, INFINITE )) && !params->done) + { + if (params->lock) + { + ret = HeapLock( params->heap ); + ok( ret, "HeapLock failed, error %lu\n", GetLastError() ); + } + + ptr = HeapAlloc( params->heap, params->flags, 0 ); + ok( !!ptr, "HeapAlloc failed, error %lu\n", GetLastError() ); + ret = HeapFree( params->heap, params->flags, ptr ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + + if (params->lock) + { + ret = HeapUnlock( params->heap ); + ok( ret, "HeapUnlock failed, error %lu\n", GetLastError() ); + } + + SetEvent( params->ready_event ); + } + ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + + return 0; +} + + static void test_HeapCreate(void) { static const BYTE buffer[512] = {0}; SIZE_T alloc_size = 0x8000 * sizeof(void *), size, i; + struct heap_thread_params thread_params = {0}; PROCESS_HEAP_ENTRY entry, entries[256]; - HANDLE heap, heap1, heaps[8]; + HANDLE heap, heap1, heaps[8], thread; BYTE *ptr, *ptr1, *ptrs[128]; DWORD heap_count, count; ULONG compat_info; UINT_PTR align; + DWORD res; BOOL ret; + thread_params.ready_event = CreateEventW( NULL, FALSE, FALSE, NULL ); + ok( !!thread_params.ready_event, "CreateEventW failed, error %lu\n", GetLastError() ); + thread_params.start_event = CreateEventW( NULL, FALSE, FALSE, NULL ); + ok( !!thread_params.start_event, "CreateEventW failed, error %lu\n", GetLastError() ); + thread = CreateThread( NULL, 0, heap_thread_proc, &thread_params, 0, NULL ); + ok( !!thread, "CreateThread failed, error %lu\n", GetLastError() ); + res = WaitForSingleObject( thread_params.ready_event, INFINITE ); + ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + heap_count = GetProcessHeaps( 0, NULL ); ok( heap_count <= 6, "GetProcessHeaps returned %lu\n", heap_count ); @@ -879,6 +937,148 @@ static void test_HeapCreate(void) ret = HeapDestroy( heap ); ok( ret, "HeapDestroy failed, error %lu\n", GetLastError() ); + + + /* check HEAP_NO_SERIALIZE HeapCreate flag effect */ + + heap = HeapCreate( HEAP_NO_SERIALIZE, 0, 0 ); + ok( !!heap, "HeapCreate failed, error %lu\n", GetLastError() ); + ok( !((ULONG_PTR)heap & 0xffff), "wrong heap alignment\n" ); + + ret = HeapLock( heap ); + ok( ret, "HeapLock failed, error %lu\n", GetLastError() ); + thread_params.heap = heap; + thread_params.lock = TRUE; + thread_params.flags = 0; + SetEvent( thread_params.start_event ); + res = WaitForSingleObject( thread_params.ready_event, 100 ); + todo_wine + ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + ret = HeapUnlock( heap ); + ok( ret, "HeapUnlock failed, error %lu\n", GetLastError() ); + if (res) WaitForSingleObject( thread_params.ready_event, 100 ); + + ret = HeapLock( heap ); + ok( ret, "HeapLock failed, error %lu\n", GetLastError() ); + thread_params.heap = heap; + thread_params.lock = FALSE; + thread_params.flags = 0; + SetEvent( thread_params.start_event ); + res = WaitForSingleObject( thread_params.ready_event, 100 ); + ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + ret = HeapUnlock( heap ); + ok( ret, "HeapUnlock failed, error %lu\n", GetLastError() ); + + ret = HeapDestroy( heap ); + ok( ret, "HeapDestroy failed, error %lu\n", GetLastError() ); + + + /* check HEAP_NO_SERIALIZE HeapAlloc / HeapFree flag effect */ + + heap = HeapCreate( 0, 0, 0 ); + ok( !!heap, "HeapCreate failed, error %lu\n", GetLastError() ); + ok( !((ULONG_PTR)heap & 0xffff), "wrong heap alignment\n" ); + + ret = HeapLock( heap ); + ok( ret, "HeapLock failed, error %lu\n", GetLastError() ); + thread_params.heap = heap; + thread_params.lock = TRUE; + thread_params.flags = 0; + SetEvent( thread_params.start_event ); + res = WaitForSingleObject( thread_params.ready_event, 100 ); + ok( res == WAIT_TIMEOUT, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + ret = HeapUnlock( heap ); + ok( ret, "HeapUnlock failed, error %lu\n", GetLastError() ); + res = WaitForSingleObject( thread_params.ready_event, 100 ); + ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + + ret = HeapLock( heap ); + ok( ret, "HeapLock failed, error %lu\n", GetLastError() ); + thread_params.heap = heap; + thread_params.lock = FALSE; + thread_params.flags = 0; + SetEvent( thread_params.start_event ); + res = WaitForSingleObject( thread_params.ready_event, 100 ); + ok( res == WAIT_TIMEOUT, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + ret = HeapUnlock( heap ); + ok( ret, "HeapUnlock failed, error %lu\n", GetLastError() ); + res = WaitForSingleObject( thread_params.ready_event, 100 ); + ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + + ret = HeapLock( heap ); + ok( ret, "HeapLock failed, error %lu\n", GetLastError() ); + thread_params.heap = heap; + thread_params.lock = FALSE; + thread_params.flags = HEAP_NO_SERIALIZE; + SetEvent( thread_params.start_event ); + res = WaitForSingleObject( thread_params.ready_event, 100 ); + ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + ret = HeapUnlock( heap ); + ok( ret, "HeapUnlock failed, error %lu\n", GetLastError() ); + + ret = HeapDestroy( heap ); + ok( ret, "HeapDestroy failed, error %lu\n", GetLastError() ); + + + /* check LFH heap locking */ + + heap = HeapCreate( 0, 0, 0 ); + ok( !!heap, "HeapCreate failed, error %lu\n", GetLastError() ); + ok( !((ULONG_PTR)heap & 0xffff), "wrong heap alignment\n" ); + + ret = pHeapQueryInformation( heap, HeapCompatibilityInformation, &compat_info, sizeof(compat_info), &size ); + ok( ret, "HeapQueryInformation failed, error %lu\n", GetLastError() ); + ok( compat_info == 0, "got HeapCompatibilityInformation %lu\n", compat_info ); + + for (i = 0; i < 0x12; i++) ptrs[i] = pHeapAlloc( heap, 0, 0 ); + for (i = 0; i < 0x12; i++) HeapFree( heap, 0, ptrs[i] ); + + ret = pHeapQueryInformation( heap, HeapCompatibilityInformation, &compat_info, sizeof(compat_info), &size ); + ok( ret, "HeapQueryInformation failed, error %lu\n", GetLastError() ); + todo_wine + ok( compat_info == 2, "got HeapCompatibilityInformation %lu\n", compat_info ); + + /* locking is serialized */ + + ret = HeapLock( heap ); + ok( ret, "HeapLock failed, error %lu\n", GetLastError() ); + thread_params.heap = heap; + thread_params.lock = TRUE; + thread_params.flags = 0; + SetEvent( thread_params.start_event ); + res = WaitForSingleObject( thread_params.ready_event, 100 ); + ok( res == WAIT_TIMEOUT, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + ret = HeapUnlock( heap ); + ok( ret, "HeapUnlock failed, error %lu\n", GetLastError() ); + res = WaitForSingleObject( thread_params.ready_event, 100 ); + ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + + /* but allocation is not */ + + ret = HeapLock( heap ); + ok( ret, "HeapLock failed, error %lu\n", GetLastError() ); + thread_params.heap = heap; + thread_params.lock = FALSE; + thread_params.flags = 0; + SetEvent( thread_params.start_event ); + res = WaitForSingleObject( thread_params.ready_event, 100 ); + todo_wine + ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + ret = HeapUnlock( heap ); + ok( ret, "HeapUnlock failed, error %lu\n", GetLastError() ); + if (res) res = WaitForSingleObject( thread_params.ready_event, 100 ); + + ret = HeapDestroy( heap ); + ok( ret, "HeapDestroy failed, error %lu\n", GetLastError() ); + + + thread_params.done = TRUE; + SetEvent( thread_params.start_event ); + res = WaitForSingleObject( thread, INFINITE ); + ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); + CloseHandle( thread_params.start_event ); + CloseHandle( thread_params.ready_event ); + CloseHandle( thread ); } -- 2.35.1
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 107 ++++++++++++++++++++++++---------------------- 1 file changed, 56 insertions(+), 51 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 7f41088347d..c3689ecca9e 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -162,7 +162,7 @@ typedef struct tagHEAP DWORD magic; /* Magic number */ DWORD pending_pos; /* Position in pending free requests ring */ ARENA_INUSE **pending_free; /* Ring buffer for pending free requests */ - RTL_CRITICAL_SECTION critSection; /* Critical section for serialization */ + RTL_CRITICAL_SECTION cs; FREE_LIST_ENTRY *freeList; /* Free lists */ } HEAP; @@ -324,13 +324,24 @@ static inline ULONG get_protection_type( DWORD flags ) return (flags & HEAP_CREATE_ENABLE_EXECUTE) ? PAGE_EXECUTE_READWRITE : PAGE_READWRITE; } -static RTL_CRITICAL_SECTION_DEBUG process_heap_critsect_debug = +static RTL_CRITICAL_SECTION_DEBUG process_heap_cs_debug = { 0, 0, NULL, /* will be set later */ - { &process_heap_critsect_debug.ProcessLocksList, &process_heap_critsect_debug.ProcessLocksList }, + { &process_heap_cs_debug.ProcessLocksList, &process_heap_cs_debug.ProcessLocksList }, 0, 0, { (DWORD_PTR)(__FILE__ ": main process heap section") } }; +static void heap_lock( HEAP *heap, DWORD flags ) +{ + if ((flags | heap->flags) & HEAP_NO_SERIALIZE) return; + RtlEnterCriticalSection( &heap->cs ); +} + +static void heap_unlock( HEAP *heap, DWORD flags ) +{ + if ((flags | heap->flags) & HEAP_NO_SERIALIZE) return; + RtlLeaveCriticalSection( &heap->cs ); +} /*********************************************************************** * HEAP_Dump @@ -958,31 +969,31 @@ static SUBHEAP *HEAP_CreateSubHeap( HEAP *heap, LPVOID address, DWORD flags, if (!processHeap) /* do it by hand to avoid memory allocations */ { - heap->critSection.DebugInfo = &process_heap_critsect_debug; - heap->critSection.LockCount = -1; - heap->critSection.RecursionCount = 0; - heap->critSection.OwningThread = 0; - heap->critSection.LockSemaphore = 0; - heap->critSection.SpinCount = 0; - process_heap_critsect_debug.CriticalSection = &heap->critSection; + heap->cs.DebugInfo = &process_heap_cs_debug; + heap->cs.LockCount = -1; + heap->cs.RecursionCount = 0; + heap->cs.OwningThread = 0; + heap->cs.LockSemaphore = 0; + heap->cs.SpinCount = 0; + process_heap_cs_debug.CriticalSection = &heap->cs; } else { - RtlInitializeCriticalSection( &heap->critSection ); - heap->critSection.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": HEAP.critSection"); + RtlInitializeCriticalSection( &heap->cs ); + heap->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": heap.cs"); } if (heap->shared) { /* let's assume that only one thread at a time will try to do this */ - HANDLE sem = heap->critSection.LockSemaphore; + HANDLE sem = heap->cs.LockSemaphore; if (!sem) NtCreateSemaphore( &sem, SEMAPHORE_ALL_ACCESS, NULL, 0, 1 ); NtDuplicateObject( NtCurrentProcess(), sem, NtCurrentProcess(), &sem, 0, 0, DUPLICATE_MAKE_GLOBAL | DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE ); - heap->critSection.LockSemaphore = sem; - RtlFreeHeap( processHeap, 0, heap->critSection.DebugInfo ); - heap->critSection.DebugInfo = NULL; + heap->cs.LockSemaphore = sem; + RtlFreeHeap( processHeap, 0, heap->cs.DebugInfo ); + heap->cs.DebugInfo = NULL; } } @@ -1339,11 +1350,7 @@ static BOOL HEAP_IsRealArena( HEAP *heapPtr, /* [in] ptr to the heap */ BOOL ret = FALSE; const ARENA_LARGE *large_arena; - flags &= HEAP_NO_SERIALIZE; - flags |= heapPtr->flags; - /* calling HeapLock may result in infinite recursion, so do the critsect directly */ - if (!(flags & HEAP_NO_SERIALIZE)) - RtlEnterCriticalSection( &heapPtr->critSection ); + heap_lock( heapPtr, flags ); if (block) /* only check this single memory block */ { @@ -1389,7 +1396,7 @@ static BOOL HEAP_IsRealArena( HEAP *heapPtr, /* [in] ptr to the heap */ ret = TRUE; done: - if (!(flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + heap_unlock( heapPtr, flags ); return ret; } @@ -1569,9 +1576,9 @@ HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T c if (processHeap) { HEAP *heapPtr = subheap->heap; - RtlEnterCriticalSection( &processHeap->critSection ); + RtlEnterCriticalSection( &processHeap->cs ); list_add_head( &processHeap->entry, &heapPtr->entry ); - RtlLeaveCriticalSection( &processHeap->critSection ); + RtlLeaveCriticalSection( &processHeap->cs ); } else if (!addr) { @@ -1615,12 +1622,12 @@ HANDLE WINAPI RtlDestroyHeap( HANDLE heap ) if (heap == processHeap) return heap; /* cannot delete the main process heap */ /* remove it from the per-process list */ - RtlEnterCriticalSection( &processHeap->critSection ); + RtlEnterCriticalSection( &processHeap->cs ); list_remove( &heapPtr->entry ); - RtlLeaveCriticalSection( &processHeap->critSection ); + RtlLeaveCriticalSection( &processHeap->cs ); - heapPtr->critSection.DebugInfo->Spare[0] = 0; - RtlDeleteCriticalSection( &heapPtr->critSection ); + heapPtr->cs.DebugInfo->Spare[0] = 0; + RtlDeleteCriticalSection( &heapPtr->cs ); LIST_FOR_EACH_ENTRY_SAFE( arena, arena_next, &heapPtr->large_list, ARENA_LARGE, entry ) { @@ -1685,12 +1692,12 @@ void * WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE heap, ULONG flags, SIZE_ } if (rounded_size < HEAP_MIN_DATA_SIZE) rounded_size = HEAP_MIN_DATA_SIZE; - if (!(flags & HEAP_NO_SERIALIZE)) RtlEnterCriticalSection( &heapPtr->critSection ); + heap_lock( heapPtr, flags ); if (rounded_size >= HEAP_MIN_LARGE_BLOCK_SIZE && (flags & HEAP_GROWABLE)) { void *ret = allocate_large_block( heap, flags, size ); - if (!(flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + heap_unlock( heapPtr, flags ); if (!ret && (flags & HEAP_GENERATE_EXCEPTIONS)) RtlRaiseStatus( STATUS_NO_MEMORY ); TRACE("(%p,%08x,%08lx): returning %p\n", heap, flags, size, ret ); return ret; @@ -1702,7 +1709,7 @@ void * WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE heap, ULONG flags, SIZE_ { TRACE("(%p,%08x,%08lx): returning NULL\n", heap, flags, size ); - if (!(flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + heap_unlock( heapPtr, flags ); if (flags & HEAP_GENERATE_EXCEPTIONS) RtlRaiseStatus( STATUS_NO_MEMORY ); return NULL; } @@ -1728,7 +1735,7 @@ void * WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE heap, ULONG flags, SIZE_ notify_alloc( pInUse + 1, size, flags & HEAP_ZERO_MEMORY ); initialize_block( pInUse + 1, size, pInUse->unused_bytes, flags ); - if (!(flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + heap_unlock( heapPtr, flags ); TRACE("(%p,%08x,%08lx): returning %p\n", heap, flags, size, pInUse + 1 ); return pInUse + 1; @@ -1766,9 +1773,7 @@ BOOLEAN WINAPI DECLSPEC_HOTPATCH RtlFreeHeap( HANDLE heap, ULONG flags, void *pt return FALSE; } - flags &= HEAP_NO_SERIALIZE; - flags |= heapPtr->flags; - if (!(flags & HEAP_NO_SERIALIZE)) RtlEnterCriticalSection( &heapPtr->critSection ); + heap_lock( heapPtr, flags ); /* Inform valgrind we are trying to free memory, so it can throw up an error message */ notify_free( ptr ); @@ -1782,12 +1787,12 @@ BOOLEAN WINAPI DECLSPEC_HOTPATCH RtlFreeHeap( HANDLE heap, ULONG flags, void *pt else HEAP_MakeInUseBlockFree( subheap, pInUse ); - if (!(flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + heap_unlock( heapPtr, flags ); TRACE("(%p,%08x,%p): returning TRUE\n", heap, flags, ptr ); return TRUE; error: - if (!(flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + heap_unlock( heapPtr, flags ); RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_INVALID_PARAMETER ); TRACE("(%p,%08x,%p): returning FALSE\n", heap, flags, ptr ); return FALSE; @@ -1829,7 +1834,7 @@ PVOID WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, PVOID ptr, SIZE_T size flags &= HEAP_GENERATE_EXCEPTIONS | HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY | HEAP_REALLOC_IN_PLACE_ONLY; flags |= heapPtr->flags; - if (!(flags & HEAP_NO_SERIALIZE)) RtlEnterCriticalSection( &heapPtr->critSection ); + heap_lock( heapPtr, flags ); rounded_size = ROUND_SIZE(size) + HEAP_TAIL_EXTRA_SIZE(flags); if (rounded_size < size) goto oom; /* overflow */ @@ -1923,19 +1928,19 @@ PVOID WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, PVOID ptr, SIZE_T size ret = pArena + 1; done: - if (!(flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + heap_unlock( heapPtr, flags ); TRACE("(%p,%08x,%p,%08lx): returning %p\n", heap, flags, ptr, size, ret ); return ret; oom: - if (!(flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + heap_unlock( heapPtr, flags ); if (flags & HEAP_GENERATE_EXCEPTIONS) RtlRaiseStatus( STATUS_NO_MEMORY ); RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_NO_MEMORY ); TRACE("(%p,%08x,%p,%08lx): returning NULL\n", heap, flags, ptr, size ); return NULL; error: - if (!(flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + heap_unlock( heapPtr, flags ); RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_INVALID_PARAMETER ); TRACE("(%p,%08x,%p,%08lx): returning NULL\n", heap, flags, ptr, size ); return NULL; @@ -1981,7 +1986,7 @@ BOOLEAN WINAPI RtlLockHeap( HANDLE heap ) { HEAP *heapPtr = HEAP_GetPtr( heap ); if (!heapPtr) return FALSE; - RtlEnterCriticalSection( &heapPtr->critSection ); + RtlEnterCriticalSection( &heapPtr->cs ); return TRUE; } @@ -2002,7 +2007,7 @@ BOOLEAN WINAPI RtlUnlockHeap( HANDLE heap ) { HEAP *heapPtr = HEAP_GetPtr( heap ); if (!heapPtr) return FALSE; - RtlLeaveCriticalSection( &heapPtr->critSection ); + RtlLeaveCriticalSection( &heapPtr->cs ); return TRUE; } @@ -2036,9 +2041,8 @@ SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr ) RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_INVALID_HANDLE ); return ~(SIZE_T)0; } - flags &= HEAP_NO_SERIALIZE; - flags |= heapPtr->flags; - if (!(flags & HEAP_NO_SERIALIZE)) RtlEnterCriticalSection( &heapPtr->critSection ); + + heap_lock( heapPtr, flags ); pArena = (const ARENA_INUSE *)ptr - 1; if (!validate_block_pointer( heapPtr, &subheap, pArena )) @@ -2055,7 +2059,8 @@ SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr ) { ret = (pArena->size & ARENA_SIZE_MASK) - pArena->unused_bytes; } - if (!(flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + + heap_unlock( heapPtr, flags ); TRACE("(%p,%08x,%p): returning %08lx\n", heap, flags, ptr, ret ); return ret; @@ -2102,7 +2107,7 @@ NTSTATUS WINAPI RtlWalkHeap( HANDLE heap, PVOID entry_ptr ) if (!heapPtr || !entry) return STATUS_INVALID_PARAMETER; - if (!(heapPtr->flags & HEAP_NO_SERIALIZE)) RtlEnterCriticalSection( &heapPtr->critSection ); + heap_lock( heapPtr, 0 ); /* FIXME: enumerate large blocks too */ @@ -2207,7 +2212,7 @@ NTSTATUS WINAPI RtlWalkHeap( HANDLE heap, PVOID entry_ptr ) if (TRACE_ON(heap)) HEAP_DumpEntry(entry); HW_end: - if (!(heapPtr->flags & HEAP_NO_SERIALIZE)) RtlLeaveCriticalSection( &heapPtr->critSection ); + heap_unlock( heapPtr, 0 ); return ret; } @@ -2230,7 +2235,7 @@ ULONG WINAPI RtlGetProcessHeaps( ULONG count, HANDLE *heaps ) ULONG total = 1; /* main heap */ struct list *ptr; - RtlEnterCriticalSection( &processHeap->critSection ); + RtlEnterCriticalSection( &processHeap->cs ); LIST_FOR_EACH( ptr, &processHeap->entry ) total++; if (total <= count) { @@ -2238,7 +2243,7 @@ ULONG WINAPI RtlGetProcessHeaps( ULONG count, HANDLE *heaps ) LIST_FOR_EACH( ptr, &processHeap->entry ) *heaps++ = LIST_ENTRY( ptr, HEAP, entry ); } - RtlLeaveCriticalSection( &processHeap->critSection ); + RtlLeaveCriticalSection( &processHeap->cs ); return total; } -- 2.35.1
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/kernel32/tests/heap.c | 2 -- dlls/ntdll/heap.c | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 28cbee6ea83..93392ad3a3c 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -952,11 +952,9 @@ static void test_HeapCreate(void) thread_params.flags = 0; SetEvent( thread_params.start_event ); res = WaitForSingleObject( thread_params.ready_event, 100 ); - todo_wine ok( !res, "WaitForSingleObject returned %#lx, error %lu\n", res, GetLastError() ); ret = HeapUnlock( heap ); ok( ret, "HeapUnlock failed, error %lu\n", GetLastError() ); - if (res) WaitForSingleObject( thread_params.ready_event, 100 ); ret = HeapLock( heap ); ok( ret, "HeapLock failed, error %lu\n", GetLastError() ); diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index c3689ecca9e..37e77cf1d23 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -1986,7 +1986,7 @@ BOOLEAN WINAPI RtlLockHeap( HANDLE heap ) { HEAP *heapPtr = HEAP_GetPtr( heap ); if (!heapPtr) return FALSE; - RtlEnterCriticalSection( &heapPtr->cs ); + heap_lock( heapPtr, 0 ); return TRUE; } @@ -2007,7 +2007,7 @@ BOOLEAN WINAPI RtlUnlockHeap( HANDLE heap ) { HEAP *heapPtr = HEAP_GetPtr( heap ); if (!heapPtr) return FALSE; - RtlLeaveCriticalSection( &heapPtr->cs ); + heap_unlock( heapPtr, 0 ); return TRUE; } -- 2.35.1
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 912 ++++++++++++++++++++++------------------------ 1 file changed, 428 insertions(+), 484 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 37e77cf1d23..b84fa8dd42c 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -181,8 +181,6 @@ typedef struct tagHEAP static HEAP *processHeap; /* main process heap */ -static BOOL HEAP_IsRealArena( HEAP *heapPtr, DWORD flags, LPCVOID block, BOOL quiet ); - /* mark a block of memory as free for debugging purposes */ static inline void mark_block_free( void *ptr, SIZE_T size, DWORD flags ) { @@ -343,10 +341,7 @@ static void heap_unlock( HEAP *heap, DWORD flags ) RtlLeaveCriticalSection( &heap->cs ); } -/*********************************************************************** - * HEAP_Dump - */ -static void HEAP_Dump( HEAP *heap ) +static void heap_dump( HEAP *heap ) { unsigned int i; SUBHEAP *subheap; @@ -412,8 +407,7 @@ static void HEAP_Dump( HEAP *heap ) } } - -static void HEAP_DumpEntry( LPPROCESS_HEAP_ENTRY entry ) +static void heap_dump_entry( PROCESS_HEAP_ENTRY *entry ) { WORD rem_flags; TRACE( "Dumping entry %p\n", entry ); @@ -454,6 +448,426 @@ static void HEAP_DumpEntry( LPPROCESS_HEAP_ENTRY entry ) } } +static SUBHEAP *find_subheap( const HEAP *heap, const void *ptr ) +{ + SUBHEAP *sub; + + LIST_FOR_EACH_ENTRY( sub, &heap->subheap_list, SUBHEAP, entry ) + if ((ptr >= sub->base) && ((const char *)ptr < (const char *)sub->base + sub->size - sizeof(ARENA_INUSE))) + return sub; + + return NULL; +} + +static ARENA_LARGE *find_large_block( HEAP *heap, const void *ptr ) +{ + ARENA_LARGE *arena; + + LIST_FOR_EACH_ENTRY( arena, &heap->large_list, ARENA_LARGE, entry ) + if (ptr == arena + 1) return arena; + + return NULL; +} + +static BOOL validate_large_arena( HEAP *heap, const ARENA_LARGE *arena, BOOL quiet ) +{ + DWORD flags = heap->flags; + + if ((ULONG_PTR)arena % page_size) + { + if (quiet == NOISY) + { + ERR( "Heap %p: invalid large arena pointer %p\n", heap, arena ); + if (TRACE_ON(heap)) heap_dump( heap ); + } + else if (WARN_ON(heap)) + { + WARN( "Heap %p: unaligned arena pointer %p\n", heap, arena ); + if (TRACE_ON(heap)) heap_dump( heap ); + } + return FALSE; + } + if (arena->size != ARENA_LARGE_SIZE || arena->magic != ARENA_LARGE_MAGIC) + { + if (quiet == NOISY) + { + ERR( "Heap %p: invalid large arena %p values %x/%x\n", + heap, arena, arena->size, arena->magic ); + if (TRACE_ON(heap)) heap_dump( heap ); + } + else if (WARN_ON(heap)) + { + WARN( "Heap %p: invalid large arena %p values %x/%x\n", + heap, arena, arena->size, arena->magic ); + if (TRACE_ON(heap)) heap_dump( heap ); + } + return FALSE; + } + if (arena->data_size > arena->block_size - sizeof(*arena)) + { + ERR( "Heap %p: invalid large arena %p size %lx/%lx\n", + heap, arena, arena->data_size, arena->block_size ); + return FALSE; + } + if (flags & HEAP_TAIL_CHECKING_ENABLED) + { + SIZE_T i, unused = arena->block_size - sizeof(*arena) - arena->data_size; + const unsigned char *data = (const unsigned char *)(arena + 1) + arena->data_size; + + for (i = 0; i < unused; i++) + { + if (data[i] == ARENA_TAIL_FILLER) continue; + ERR("Heap %p: block %p tail overwritten at %p (byte %lu/%lu == 0x%02x)\n", + heap, arena + 1, data + i, i, unused, data[i] ); + return FALSE; + } + } + return TRUE; +} + +static BOOL HEAP_IsValidArenaPtr( const HEAP *heap, const ARENA_FREE *ptr ) +{ + unsigned int i; + const SUBHEAP *subheap = find_subheap( heap, ptr ); + if (!subheap) return FALSE; + if ((const char *)ptr >= (const char *)subheap->base + subheap->headerSize) return TRUE; + if (subheap != &heap->subheap) return FALSE; + for (i = 0; i < HEAP_NB_FREE_LISTS; i++) + if (ptr == &heap->freeList[i].arena) return TRUE; + return FALSE; +} + +static BOOL validate_used_block( const SUBHEAP *subheap, const ARENA_INUSE *pArena, BOOL quiet ) +{ + SIZE_T size; + DWORD i, flags = subheap->heap->flags; + const char *heapEnd = (const char *)subheap->base + subheap->size; + + /* Check for unaligned pointers */ + if ((ULONG_PTR)pArena % ALIGNMENT != ARENA_OFFSET) + { + if (quiet == NOISY) + { + ERR( "Heap %p: unaligned arena pointer %p\n", subheap->heap, pArena ); + if (TRACE_ON(heap)) heap_dump( subheap->heap ); + } + else if (WARN_ON(heap)) + { + WARN( "Heap %p: unaligned arena pointer %p\n", subheap->heap, pArena ); + if (TRACE_ON(heap)) heap_dump( subheap->heap ); + } + return FALSE; + } + + /* Check magic number */ + if (pArena->magic != ARENA_INUSE_MAGIC && pArena->magic != ARENA_PENDING_MAGIC) + { + if (quiet == NOISY) + { + ERR("Heap %p: invalid in-use arena magic %08x for %p\n", subheap->heap, pArena->magic, pArena ); + if (TRACE_ON(heap)) heap_dump( subheap->heap ); + } + else if (WARN_ON(heap)) + { + WARN("Heap %p: invalid in-use arena magic %08x for %p\n", subheap->heap, pArena->magic, pArena ); + if (TRACE_ON(heap)) heap_dump( subheap->heap ); + } + return FALSE; + } + /* Check size flags */ + if (pArena->size & ARENA_FLAG_FREE) + { + ERR("Heap %p: bad flags %08x for in-use arena %p\n", + subheap->heap, pArena->size & ~ARENA_SIZE_MASK, pArena ); + return FALSE; + } + /* Check arena size */ + size = pArena->size & ARENA_SIZE_MASK; + if ((const char *)(pArena + 1) + size > heapEnd || + (const char *)(pArena + 1) + size < (const char *)(pArena + 1)) + { + ERR("Heap %p: bad size %08lx for in-use arena %p\n", subheap->heap, size, pArena ); + return FALSE; + } + /* Check next arena PREV_FREE flag */ + if (((const char *)(pArena + 1) + size < heapEnd) && + (*(const DWORD *)((const char *)(pArena + 1) + size) & ARENA_FLAG_PREV_FREE)) + { + ERR("Heap %p: in-use arena %p next block %p has PREV_FREE flag %x\n", + subheap->heap, pArena, (const char *)(pArena + 1) + size,*(const DWORD *)((const char *)(pArena + 1) + size) ); + return FALSE; + } + /* Check prev free arena */ + if (pArena->size & ARENA_FLAG_PREV_FREE) + { + const ARENA_FREE *pPrev = *((const ARENA_FREE * const*)pArena - 1); + /* Check prev pointer */ + if (!HEAP_IsValidArenaPtr( subheap->heap, pPrev )) + { + ERR("Heap %p: bad back ptr %p for arena %p\n", + subheap->heap, pPrev, pArena ); + return FALSE; + } + /* Check that prev arena is free */ + if (!(pPrev->size & ARENA_FLAG_FREE) || + (pPrev->magic != ARENA_FREE_MAGIC)) + { + ERR("Heap %p: prev arena %p invalid for in-use %p\n", + subheap->heap, pPrev, pArena ); + return FALSE; + } + /* Check that prev arena is really the previous block */ + if ((const char *)(pPrev + 1) + (pPrev->size & ARENA_SIZE_MASK) != (const char *)pArena) + { + ERR("Heap %p: prev arena %p is not prev for in-use %p\n", + subheap->heap, pPrev, pArena ); + return FALSE; + } + } + /* Check unused size */ + if (pArena->unused_bytes > size) + { + ERR("Heap %p: invalid unused size %08x/%08lx\n", subheap->heap, pArena->unused_bytes, size ); + return FALSE; + } + /* Check unused bytes */ + if (pArena->magic == ARENA_PENDING_MAGIC) + { + const DWORD *ptr = (const DWORD *)(pArena + 1); + const DWORD *end = (const DWORD *)((const char *)ptr + size); + + while (ptr < end) + { + if (*ptr != ARENA_FREE_FILLER) + { + ERR("Heap %p: free block %p overwritten at %p by %08x\n", + subheap->heap, pArena + 1, ptr, *ptr ); + if (!*ptr) { heap_dump( subheap->heap ); DbgBreakPoint(); } + return FALSE; + } + ptr++; + } + } + else if (flags & HEAP_TAIL_CHECKING_ENABLED) + { + const unsigned char *data = (const unsigned char *)(pArena + 1) + size - pArena->unused_bytes; + + for (i = 0; i < pArena->unused_bytes; i++) + { + if (data[i] == ARENA_TAIL_FILLER) continue; + ERR("Heap %p: block %p tail overwritten at %p (byte %u/%u == 0x%02x)\n", + subheap->heap, pArena + 1, data + i, i, pArena->unused_bytes, data[i] ); + return FALSE; + } + } + return TRUE; +} + +static BOOL validate_free_block( SUBHEAP *subheap, ARENA_FREE *pArena ) +{ + DWORD flags = subheap->heap->flags; + SIZE_T size; + ARENA_FREE *prev, *next; + char *heapEnd = (char *)subheap->base + subheap->size; + + /* Check for unaligned pointers */ + if ((ULONG_PTR)pArena % ALIGNMENT != ARENA_OFFSET) + { + ERR("Heap %p: unaligned arena pointer %p\n", subheap->heap, pArena ); + return FALSE; + } + + /* Check magic number */ + if (pArena->magic != ARENA_FREE_MAGIC) + { + ERR("Heap %p: invalid free arena magic %08x for %p\n", subheap->heap, pArena->magic, pArena ); + return FALSE; + } + /* Check size flags */ + if (!(pArena->size & ARENA_FLAG_FREE) || + (pArena->size & ARENA_FLAG_PREV_FREE)) + { + ERR("Heap %p: bad flags %08x for free arena %p\n", + subheap->heap, pArena->size & ~ARENA_SIZE_MASK, pArena ); + return FALSE; + } + /* Check arena size */ + size = pArena->size & ARENA_SIZE_MASK; + if ((char *)(pArena + 1) + size > heapEnd) + { + ERR("Heap %p: bad size %08lx for free arena %p\n", subheap->heap, size, pArena ); + return FALSE; + } + /* Check that next pointer is valid */ + next = LIST_ENTRY( pArena->entry.next, ARENA_FREE, entry ); + if (!HEAP_IsValidArenaPtr( subheap->heap, next )) + { + ERR("Heap %p: bad next ptr %p for arena %p\n", + subheap->heap, next, pArena ); + return FALSE; + } + /* Check that next arena is free */ + if (!(next->size & ARENA_FLAG_FREE) || (next->magic != ARENA_FREE_MAGIC)) + { + ERR("Heap %p: next arena %p invalid for %p\n", + subheap->heap, next, pArena ); + return FALSE; + } + /* Check that prev pointer is valid */ + prev = LIST_ENTRY( pArena->entry.prev, ARENA_FREE, entry ); + if (!HEAP_IsValidArenaPtr( subheap->heap, prev )) + { + ERR("Heap %p: bad prev ptr %p for arena %p\n", + subheap->heap, prev, pArena ); + return FALSE; + } + /* Check that prev arena is free */ + if (!(prev->size & ARENA_FLAG_FREE) || (prev->magic != ARENA_FREE_MAGIC)) + { + /* this often means that the prev arena got overwritten + * by a memory write before that prev arena */ + ERR("Heap %p: prev arena %p invalid for %p\n", + subheap->heap, prev, pArena ); + return FALSE; + } + /* Check that next block has PREV_FREE flag */ + if ((char *)(pArena + 1) + size < heapEnd) + { + if (!(*(DWORD *)((char *)(pArena + 1) + size) & ARENA_FLAG_PREV_FREE)) + { + ERR("Heap %p: free arena %p next block has no PREV_FREE flag\n", + subheap->heap, pArena ); + return FALSE; + } + /* Check next block back pointer */ + if (*((ARENA_FREE **)((char *)(pArena + 1) + size) - 1) != pArena) + { + ERR("Heap %p: arena %p has wrong back ptr %p\n", + subheap->heap, pArena, + *((ARENA_FREE **)((char *)(pArena+1) + size) - 1)); + return FALSE; + } + } + if (flags & HEAP_FREE_CHECKING_ENABLED) + { + DWORD *ptr = (DWORD *)(pArena + 1); + char *end = (char *)(pArena + 1) + size; + + if (end >= heapEnd) end = (char *)subheap->base + subheap->commitSize; + else end -= sizeof(ARENA_FREE *); + while (ptr < (DWORD *)end) + { + if (*ptr != ARENA_FREE_FILLER) + { + ERR("Heap %p: free block %p overwritten at %p by %08x\n", + subheap->heap, (ARENA_INUSE *)pArena + 1, ptr, *ptr ); + return FALSE; + } + ptr++; + } + } + return TRUE; +} + +static BOOL validate_block_pointer( HEAP *heap, SUBHEAP **ret_subheap, const ARENA_INUSE *arena ) +{ + SUBHEAP *subheap; + BOOL ret = FALSE; + + if (!(*ret_subheap = subheap = find_subheap( heap, arena ))) + { + ARENA_LARGE *large_arena = find_large_block( heap, arena + 1 ); + + if (!large_arena) + { + WARN( "Heap %p: pointer %p is not inside heap\n", heap, arena + 1 ); + return FALSE; + } + if ((heap->flags & HEAP_VALIDATE) && !validate_large_arena( heap, large_arena, QUIET )) + return FALSE; + return TRUE; + } + + if ((const char *)arena < (char *)subheap->base + subheap->headerSize) + WARN( "Heap %p: pointer %p is inside subheap %p header\n", subheap->heap, arena + 1, subheap ); + else if (subheap->heap->flags & HEAP_VALIDATE) /* do the full validation */ + ret = validate_used_block( subheap, arena, QUIET ); + else if ((ULONG_PTR)arena % ALIGNMENT != ARENA_OFFSET) + WARN( "Heap %p: unaligned arena pointer %p\n", subheap->heap, arena ); + else if (arena->magic == ARENA_PENDING_MAGIC) + WARN( "Heap %p: block %p used after free\n", subheap->heap, arena + 1 ); + else if (arena->magic != ARENA_INUSE_MAGIC) + WARN( "Heap %p: invalid in-use arena magic %08x for %p\n", subheap->heap, arena->magic, arena ); + else if (arena->size & ARENA_FLAG_FREE) + ERR( "Heap %p: bad flags %08x for in-use arena %p\n", + subheap->heap, arena->size & ~ARENA_SIZE_MASK, arena ); + else if ((const char *)(arena + 1) + (arena->size & ARENA_SIZE_MASK) > (const char *)subheap->base + subheap->size || + (const char *)(arena + 1) + (arena->size & ARENA_SIZE_MASK) < (const char *)(arena + 1)) + ERR( "Heap %p: bad size %08x for in-use arena %p\n", + subheap->heap, arena->size & ARENA_SIZE_MASK, arena ); + else + ret = TRUE; + + return ret; +} + +static BOOL heap_validate( HEAP *heapPtr, DWORD flags, LPCVOID block, BOOL quiet ) +{ + SUBHEAP *subheap; + BOOL ret = FALSE; + const ARENA_LARGE *large_arena; + + heap_lock( heapPtr, flags ); + + if (block) /* only check this single memory block */ + { + const ARENA_INUSE *arena = (const ARENA_INUSE *)block - 1; + + if (!(subheap = find_subheap( heapPtr, arena )) || + ((const char *)arena < (char *)subheap->base + subheap->headerSize)) + { + if (!(large_arena = find_large_block( heapPtr, block ))) + { + if (quiet == NOISY) + ERR("Heap %p: block %p is not inside heap\n", heapPtr, block ); + else if (WARN_ON(heap)) + WARN("Heap %p: block %p is not inside heap\n", heapPtr, block ); + } + else ret = validate_large_arena( heapPtr, large_arena, quiet ); + } + else ret = validate_used_block( subheap, arena, quiet ); + goto done; + } + + LIST_FOR_EACH_ENTRY( subheap, &heapPtr->subheap_list, SUBHEAP, entry ) + { + char *ptr = (char *)subheap->base + subheap->headerSize; + while (ptr < (char *)subheap->base + subheap->size) + { + if (*(DWORD *)ptr & ARENA_FLAG_FREE) + { + if (!validate_free_block( subheap, (ARENA_FREE *)ptr )) goto done; + ptr += sizeof(ARENA_FREE) + (*(DWORD *)ptr & ARENA_SIZE_MASK); + } + else + { + if (!validate_used_block( subheap, (ARENA_INUSE *)ptr, NOISY )) goto done; + ptr += sizeof(ARENA_INUSE) + (*(DWORD *)ptr & ARENA_SIZE_MASK); + } + } + } + + LIST_FOR_EACH_ENTRY( large_arena, &heapPtr->large_list, ARENA_LARGE, entry ) + if (!validate_large_arena( heapPtr, large_arena, quiet )) goto done; + + ret = TRUE; + +done: + heap_unlock( heapPtr, flags ); + return ret; +} + + /*********************************************************************** * HEAP_GetPtr * RETURNS @@ -469,11 +883,11 @@ static HEAP *HEAP_GetPtr( ERR("Invalid heap %p!\n", heap ); return NULL; } - if ((heapPtr->flags & HEAP_VALIDATE_ALL) && !HEAP_IsRealArena( heapPtr, 0, NULL, NOISY )) + if ((heapPtr->flags & HEAP_VALIDATE_ALL) && !heap_validate( heapPtr, 0, NULL, NOISY )) { if (TRACE_ON(heap)) { - HEAP_Dump( heapPtr ); + heap_dump( heapPtr ); assert( FALSE ); } return NULL; @@ -506,27 +920,6 @@ static inline void HEAP_InsertFreeBlock( HEAP *heap, ARENA_FREE *pArena, BOOL la } -/*********************************************************************** - * HEAP_FindSubHeap - * Find the sub-heap containing a given address. - * - * RETURNS - * Pointer: Success - * NULL: Failure - */ -static SUBHEAP *HEAP_FindSubHeap( - const HEAP *heap, /* [in] Heap pointer */ - LPCVOID ptr ) /* [in] Address */ -{ - SUBHEAP *sub; - LIST_FOR_EACH_ENTRY( sub, &heap->subheap_list, SUBHEAP, entry ) - if ((ptr >= sub->base) && - ((const char *)ptr < (const char *)sub->base + sub->size - sizeof(ARENA_INUSE))) - return sub; - return NULL; -} - - /*********************************************************************** * HEAP_Commit * @@ -658,7 +1051,7 @@ static void HEAP_MakeInUseBlockFree( SUBHEAP *subheap, ARENA_INUSE *pArena ) mark_block_free( pArena + 1, pArena->size & ARENA_SIZE_MASK, heap->flags ); if (!prev) return; pArena = prev; - subheap = HEAP_FindSubHeap( heap, pArena ); + subheap = find_subheap( heap, pArena ); } /* Check if we can merge with previous block */ @@ -808,79 +1201,6 @@ static void *realloc_large_block( HEAP *heap, DWORD flags, void *ptr, SIZE_T siz } -/*********************************************************************** - * find_large_block - */ -static ARENA_LARGE *find_large_block( HEAP *heap, const void *ptr ) -{ - ARENA_LARGE *arena; - - LIST_FOR_EACH_ENTRY( arena, &heap->large_list, ARENA_LARGE, entry ) - if (ptr == arena + 1) return arena; - - return NULL; -} - - -/*********************************************************************** - * validate_large_arena - */ -static BOOL validate_large_arena( HEAP *heap, const ARENA_LARGE *arena, BOOL quiet ) -{ - DWORD flags = heap->flags; - - if ((ULONG_PTR)arena % page_size) - { - if (quiet == NOISY) - { - ERR( "Heap %p: invalid large arena pointer %p\n", heap, arena ); - if (TRACE_ON(heap)) HEAP_Dump( heap ); - } - else if (WARN_ON(heap)) - { - WARN( "Heap %p: unaligned arena pointer %p\n", heap, arena ); - if (TRACE_ON(heap)) HEAP_Dump( heap ); - } - return FALSE; - } - if (arena->size != ARENA_LARGE_SIZE || arena->magic != ARENA_LARGE_MAGIC) - { - if (quiet == NOISY) - { - ERR( "Heap %p: invalid large arena %p values %x/%x\n", - heap, arena, arena->size, arena->magic ); - if (TRACE_ON(heap)) HEAP_Dump( heap ); - } - else if (WARN_ON(heap)) - { - WARN( "Heap %p: invalid large arena %p values %x/%x\n", - heap, arena, arena->size, arena->magic ); - if (TRACE_ON(heap)) HEAP_Dump( heap ); - } - return FALSE; - } - if (arena->data_size > arena->block_size - sizeof(*arena)) - { - ERR( "Heap %p: invalid large arena %p size %lx/%lx\n", - heap, arena, arena->data_size, arena->block_size ); - return FALSE; - } - if (flags & HEAP_TAIL_CHECKING_ENABLED) - { - SIZE_T i, unused = arena->block_size - sizeof(*arena) - arena->data_size; - const unsigned char *data = (const unsigned char *)(arena + 1) + arena->data_size; - - for (i = 0; i < unused; i++) - { - if (data[i] == ARENA_TAIL_FILLER) continue; - ERR("Heap %p: block %p tail overwritten at %p (byte %lu/%lu == 0x%02x)\n", - heap, arena + 1, data + i, i, unused, data[i] ); - return FALSE; - } - } - return TRUE; -} - /*********************************************************************** * HEAP_CreateSubHeap @@ -1030,7 +1350,7 @@ static ARENA_FREE *HEAP_FindFreeBlock( HEAP *heap, SIZE_T size, sizeof(ARENA_FREE) - sizeof(ARENA_INUSE); if (arena_size >= size) { - subheap = HEAP_FindSubHeap( heap, pArena ); + subheap = find_subheap( heap, pArena ); if (!HEAP_Commit( subheap, (ARENA_INUSE *)pArena, size )) return NULL; *ppSubHeap = subheap; return pArena; @@ -1072,382 +1392,6 @@ static ARENA_FREE *HEAP_FindFreeBlock( HEAP *heap, SIZE_T size, return (ARENA_FREE *)((char *)subheap->base + subheap->headerSize); } - -/*********************************************************************** - * HEAP_IsValidArenaPtr - * - * Check that the pointer is inside the range possible for arenas. - */ -static BOOL HEAP_IsValidArenaPtr( const HEAP *heap, const ARENA_FREE *ptr ) -{ - unsigned int i; - const SUBHEAP *subheap = HEAP_FindSubHeap( heap, ptr ); - if (!subheap) return FALSE; - if ((const char *)ptr >= (const char *)subheap->base + subheap->headerSize) return TRUE; - if (subheap != &heap->subheap) return FALSE; - for (i = 0; i < HEAP_NB_FREE_LISTS; i++) - if (ptr == &heap->freeList[i].arena) return TRUE; - return FALSE; -} - - -/*********************************************************************** - * HEAP_ValidateFreeArena - */ -static BOOL HEAP_ValidateFreeArena( SUBHEAP *subheap, ARENA_FREE *pArena ) -{ - DWORD flags = subheap->heap->flags; - SIZE_T size; - ARENA_FREE *prev, *next; - char *heapEnd = (char *)subheap->base + subheap->size; - - /* Check for unaligned pointers */ - if ((ULONG_PTR)pArena % ALIGNMENT != ARENA_OFFSET) - { - ERR("Heap %p: unaligned arena pointer %p\n", subheap->heap, pArena ); - return FALSE; - } - - /* Check magic number */ - if (pArena->magic != ARENA_FREE_MAGIC) - { - ERR("Heap %p: invalid free arena magic %08x for %p\n", subheap->heap, pArena->magic, pArena ); - return FALSE; - } - /* Check size flags */ - if (!(pArena->size & ARENA_FLAG_FREE) || - (pArena->size & ARENA_FLAG_PREV_FREE)) - { - ERR("Heap %p: bad flags %08x for free arena %p\n", - subheap->heap, pArena->size & ~ARENA_SIZE_MASK, pArena ); - return FALSE; - } - /* Check arena size */ - size = pArena->size & ARENA_SIZE_MASK; - if ((char *)(pArena + 1) + size > heapEnd) - { - ERR("Heap %p: bad size %08lx for free arena %p\n", subheap->heap, size, pArena ); - return FALSE; - } - /* Check that next pointer is valid */ - next = LIST_ENTRY( pArena->entry.next, ARENA_FREE, entry ); - if (!HEAP_IsValidArenaPtr( subheap->heap, next )) - { - ERR("Heap %p: bad next ptr %p for arena %p\n", - subheap->heap, next, pArena ); - return FALSE; - } - /* Check that next arena is free */ - if (!(next->size & ARENA_FLAG_FREE) || (next->magic != ARENA_FREE_MAGIC)) - { - ERR("Heap %p: next arena %p invalid for %p\n", - subheap->heap, next, pArena ); - return FALSE; - } - /* Check that prev pointer is valid */ - prev = LIST_ENTRY( pArena->entry.prev, ARENA_FREE, entry ); - if (!HEAP_IsValidArenaPtr( subheap->heap, prev )) - { - ERR("Heap %p: bad prev ptr %p for arena %p\n", - subheap->heap, prev, pArena ); - return FALSE; - } - /* Check that prev arena is free */ - if (!(prev->size & ARENA_FLAG_FREE) || (prev->magic != ARENA_FREE_MAGIC)) - { - /* this often means that the prev arena got overwritten - * by a memory write before that prev arena */ - ERR("Heap %p: prev arena %p invalid for %p\n", - subheap->heap, prev, pArena ); - return FALSE; - } - /* Check that next block has PREV_FREE flag */ - if ((char *)(pArena + 1) + size < heapEnd) - { - if (!(*(DWORD *)((char *)(pArena + 1) + size) & ARENA_FLAG_PREV_FREE)) - { - ERR("Heap %p: free arena %p next block has no PREV_FREE flag\n", - subheap->heap, pArena ); - return FALSE; - } - /* Check next block back pointer */ - if (*((ARENA_FREE **)((char *)(pArena + 1) + size) - 1) != pArena) - { - ERR("Heap %p: arena %p has wrong back ptr %p\n", - subheap->heap, pArena, - *((ARENA_FREE **)((char *)(pArena+1) + size) - 1)); - return FALSE; - } - } - if (flags & HEAP_FREE_CHECKING_ENABLED) - { - DWORD *ptr = (DWORD *)(pArena + 1); - char *end = (char *)(pArena + 1) + size; - - if (end >= heapEnd) end = (char *)subheap->base + subheap->commitSize; - else end -= sizeof(ARENA_FREE *); - while (ptr < (DWORD *)end) - { - if (*ptr != ARENA_FREE_FILLER) - { - ERR("Heap %p: free block %p overwritten at %p by %08x\n", - subheap->heap, (ARENA_INUSE *)pArena + 1, ptr, *ptr ); - return FALSE; - } - ptr++; - } - } - return TRUE; -} - - -/*********************************************************************** - * HEAP_ValidateInUseArena - */ -static BOOL HEAP_ValidateInUseArena( const SUBHEAP *subheap, const ARENA_INUSE *pArena, BOOL quiet ) -{ - SIZE_T size; - DWORD i, flags = subheap->heap->flags; - const char *heapEnd = (const char *)subheap->base + subheap->size; - - /* Check for unaligned pointers */ - if ((ULONG_PTR)pArena % ALIGNMENT != ARENA_OFFSET) - { - if ( quiet == NOISY ) - { - ERR( "Heap %p: unaligned arena pointer %p\n", subheap->heap, pArena ); - if ( TRACE_ON(heap) ) - HEAP_Dump( subheap->heap ); - } - else if ( WARN_ON(heap) ) - { - WARN( "Heap %p: unaligned arena pointer %p\n", subheap->heap, pArena ); - if ( TRACE_ON(heap) ) - HEAP_Dump( subheap->heap ); - } - return FALSE; - } - - /* Check magic number */ - if (pArena->magic != ARENA_INUSE_MAGIC && pArena->magic != ARENA_PENDING_MAGIC) - { - if (quiet == NOISY) { - ERR("Heap %p: invalid in-use arena magic %08x for %p\n", subheap->heap, pArena->magic, pArena ); - if (TRACE_ON(heap)) - HEAP_Dump( subheap->heap ); - } else if (WARN_ON(heap)) { - WARN("Heap %p: invalid in-use arena magic %08x for %p\n", subheap->heap, pArena->magic, pArena ); - if (TRACE_ON(heap)) - HEAP_Dump( subheap->heap ); - } - return FALSE; - } - /* Check size flags */ - if (pArena->size & ARENA_FLAG_FREE) - { - ERR("Heap %p: bad flags %08x for in-use arena %p\n", - subheap->heap, pArena->size & ~ARENA_SIZE_MASK, pArena ); - return FALSE; - } - /* Check arena size */ - size = pArena->size & ARENA_SIZE_MASK; - if ((const char *)(pArena + 1) + size > heapEnd || - (const char *)(pArena + 1) + size < (const char *)(pArena + 1)) - { - ERR("Heap %p: bad size %08lx for in-use arena %p\n", subheap->heap, size, pArena ); - return FALSE; - } - /* Check next arena PREV_FREE flag */ - if (((const char *)(pArena + 1) + size < heapEnd) && - (*(const DWORD *)((const char *)(pArena + 1) + size) & ARENA_FLAG_PREV_FREE)) - { - ERR("Heap %p: in-use arena %p next block %p has PREV_FREE flag %x\n", - subheap->heap, pArena, (const char *)(pArena + 1) + size,*(const DWORD *)((const char *)(pArena + 1) + size) ); - return FALSE; - } - /* Check prev free arena */ - if (pArena->size & ARENA_FLAG_PREV_FREE) - { - const ARENA_FREE *pPrev = *((const ARENA_FREE * const*)pArena - 1); - /* Check prev pointer */ - if (!HEAP_IsValidArenaPtr( subheap->heap, pPrev )) - { - ERR("Heap %p: bad back ptr %p for arena %p\n", - subheap->heap, pPrev, pArena ); - return FALSE; - } - /* Check that prev arena is free */ - if (!(pPrev->size & ARENA_FLAG_FREE) || - (pPrev->magic != ARENA_FREE_MAGIC)) - { - ERR("Heap %p: prev arena %p invalid for in-use %p\n", - subheap->heap, pPrev, pArena ); - return FALSE; - } - /* Check that prev arena is really the previous block */ - if ((const char *)(pPrev + 1) + (pPrev->size & ARENA_SIZE_MASK) != (const char *)pArena) - { - ERR("Heap %p: prev arena %p is not prev for in-use %p\n", - subheap->heap, pPrev, pArena ); - return FALSE; - } - } - /* Check unused size */ - if (pArena->unused_bytes > size) - { - ERR("Heap %p: invalid unused size %08x/%08lx\n", subheap->heap, pArena->unused_bytes, size ); - return FALSE; - } - /* Check unused bytes */ - if (pArena->magic == ARENA_PENDING_MAGIC) - { - const DWORD *ptr = (const DWORD *)(pArena + 1); - const DWORD *end = (const DWORD *)((const char *)ptr + size); - - while (ptr < end) - { - if (*ptr != ARENA_FREE_FILLER) - { - ERR("Heap %p: free block %p overwritten at %p by %08x\n", - subheap->heap, pArena + 1, ptr, *ptr ); - if (!*ptr) { HEAP_Dump( subheap->heap ); DbgBreakPoint(); } - return FALSE; - } - ptr++; - } - } - else if (flags & HEAP_TAIL_CHECKING_ENABLED) - { - const unsigned char *data = (const unsigned char *)(pArena + 1) + size - pArena->unused_bytes; - - for (i = 0; i < pArena->unused_bytes; i++) - { - if (data[i] == ARENA_TAIL_FILLER) continue; - ERR("Heap %p: block %p tail overwritten at %p (byte %u/%u == 0x%02x)\n", - subheap->heap, pArena + 1, data + i, i, pArena->unused_bytes, data[i] ); - return FALSE; - } - } - return TRUE; -} - - -/*********************************************************************** - * HEAP_IsRealArena [Internal] - * Validates a block is a valid arena. - * - * RETURNS - * TRUE: Success - * FALSE: Failure - */ -static BOOL HEAP_IsRealArena( HEAP *heapPtr, /* [in] ptr to the heap */ - DWORD flags, /* [in] Bit flags that control access during operation */ - LPCVOID block, /* [in] Optional pointer to memory block to validate */ - BOOL quiet ) /* [in] Flag - if true, HEAP_ValidateInUseArena - * does not complain */ -{ - SUBHEAP *subheap; - BOOL ret = FALSE; - const ARENA_LARGE *large_arena; - - heap_lock( heapPtr, flags ); - - if (block) /* only check this single memory block */ - { - const ARENA_INUSE *arena = (const ARENA_INUSE *)block - 1; - - if (!(subheap = HEAP_FindSubHeap( heapPtr, arena )) || - ((const char *)arena < (char *)subheap->base + subheap->headerSize)) - { - if (!(large_arena = find_large_block( heapPtr, block ))) - { - if (quiet == NOISY) - ERR("Heap %p: block %p is not inside heap\n", heapPtr, block ); - else if (WARN_ON(heap)) - WARN("Heap %p: block %p is not inside heap\n", heapPtr, block ); - } - else ret = validate_large_arena( heapPtr, large_arena, quiet ); - } - else ret = HEAP_ValidateInUseArena( subheap, arena, quiet ); - goto done; - } - - LIST_FOR_EACH_ENTRY( subheap, &heapPtr->subheap_list, SUBHEAP, entry ) - { - char *ptr = (char *)subheap->base + subheap->headerSize; - while (ptr < (char *)subheap->base + subheap->size) - { - if (*(DWORD *)ptr & ARENA_FLAG_FREE) - { - if (!HEAP_ValidateFreeArena( subheap, (ARENA_FREE *)ptr )) goto done; - ptr += sizeof(ARENA_FREE) + (*(DWORD *)ptr & ARENA_SIZE_MASK); - } - else - { - if (!HEAP_ValidateInUseArena( subheap, (ARENA_INUSE *)ptr, NOISY )) goto done; - ptr += sizeof(ARENA_INUSE) + (*(DWORD *)ptr & ARENA_SIZE_MASK); - } - } - } - - LIST_FOR_EACH_ENTRY( large_arena, &heapPtr->large_list, ARENA_LARGE, entry ) - if (!validate_large_arena( heapPtr, large_arena, quiet )) goto done; - - ret = TRUE; - -done: - heap_unlock( heapPtr, flags ); - return ret; -} - - -/*********************************************************************** - * validate_block_pointer - * - * Minimum validation needed to catch bad parameters in heap functions. - */ -static BOOL validate_block_pointer( HEAP *heap, SUBHEAP **ret_subheap, const ARENA_INUSE *arena ) -{ - SUBHEAP *subheap; - BOOL ret = FALSE; - - if (!(*ret_subheap = subheap = HEAP_FindSubHeap( heap, arena ))) - { - ARENA_LARGE *large_arena = find_large_block( heap, arena + 1 ); - - if (!large_arena) - { - WARN( "Heap %p: pointer %p is not inside heap\n", heap, arena + 1 ); - return FALSE; - } - if ((heap->flags & HEAP_VALIDATE) && !validate_large_arena( heap, large_arena, QUIET )) - return FALSE; - return TRUE; - } - - if ((const char *)arena < (char *)subheap->base + subheap->headerSize) - WARN( "Heap %p: pointer %p is inside subheap %p header\n", subheap->heap, arena + 1, subheap ); - else if (subheap->heap->flags & HEAP_VALIDATE) /* do the full validation */ - ret = HEAP_ValidateInUseArena( subheap, arena, QUIET ); - else if ((ULONG_PTR)arena % ALIGNMENT != ARENA_OFFSET) - WARN( "Heap %p: unaligned arena pointer %p\n", subheap->heap, arena ); - else if (arena->magic == ARENA_PENDING_MAGIC) - WARN( "Heap %p: block %p used after free\n", subheap->heap, arena + 1 ); - else if (arena->magic != ARENA_INUSE_MAGIC) - WARN( "Heap %p: invalid in-use arena magic %08x for %p\n", subheap->heap, arena->magic, arena ); - else if (arena->size & ARENA_FLAG_FREE) - ERR( "Heap %p: bad flags %08x for in-use arena %p\n", - subheap->heap, arena->size & ~ARENA_SIZE_MASK, arena ); - else if ((const char *)(arena + 1) + (arena->size & ARENA_SIZE_MASK) > (const char *)subheap->base + subheap->size || - (const char *)(arena + 1) + (arena->size & ARENA_SIZE_MASK) < (const char *)(arena + 1)) - ERR( "Heap %p: bad size %08x for in-use arena %p\n", - subheap->heap, arena->size & ARENA_SIZE_MASK, arena ); - else - ret = TRUE; - - return ret; -} - static DWORD heap_flags_from_global_flag( DWORD flag ) { DWORD ret = 0; @@ -2085,7 +2029,7 @@ BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, LPCVOID ptr ) { HEAP *heapPtr = HEAP_GetPtr( heap ); if (!heapPtr) return FALSE; - return HEAP_IsRealArena( heapPtr, flags, ptr, QUIET ); + return heap_validate( heapPtr, flags, ptr, QUIET ); } @@ -2209,7 +2153,7 @@ NTSTATUS WINAPI RtlWalkHeap( HANDLE heap, PVOID entry_ptr ) (char *)currentheap->base + currentheap->size; } ret = STATUS_SUCCESS; - if (TRACE_ON(heap)) HEAP_DumpEntry(entry); + if (TRACE_ON(heap)) heap_dump_entry(entry); HW_end: heap_unlock( heapPtr, 0 ); -- 2.35.1
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 82 +++++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 38 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index b84fa8dd42c..020176347e8 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -811,35 +811,41 @@ static BOOL validate_block_pointer( HEAP *heap, SUBHEAP **ret_subheap, const ARE return ret; } -static BOOL heap_validate( HEAP *heapPtr, DWORD flags, LPCVOID block, BOOL quiet ) +static BOOL heap_validate_ptr( HEAP *heap, DWORD flags, const void *ptr, BOOL quiet ) { + const ARENA_INUSE *arena = (const ARENA_INUSE *)ptr - 1; + const ARENA_LARGE *large_arena; SUBHEAP *subheap; BOOL ret = FALSE; - const ARENA_LARGE *large_arena; - heap_lock( heapPtr, flags ); + heap_lock( heap, flags ); - if (block) /* only check this single memory block */ + if (!(subheap = find_subheap( heap, arena )) || + ((const char *)arena < (char *)subheap->base + subheap->headerSize)) { - const ARENA_INUSE *arena = (const ARENA_INUSE *)block - 1; - - if (!(subheap = find_subheap( heapPtr, arena )) || - ((const char *)arena < (char *)subheap->base + subheap->headerSize)) + if (!(large_arena = find_large_block( heap, ptr ))) { - if (!(large_arena = find_large_block( heapPtr, block ))) - { - if (quiet == NOISY) - ERR("Heap %p: block %p is not inside heap\n", heapPtr, block ); - else if (WARN_ON(heap)) - WARN("Heap %p: block %p is not inside heap\n", heapPtr, block ); - } - else ret = validate_large_arena( heapPtr, large_arena, quiet ); + if (quiet == NOISY) ERR("heap %p, ptr %p is not inside heap\n", heap, ptr ); + else if (WARN_ON(heap)) WARN("heap %p, ptr %p is not inside heap\n", heap, ptr ); } - else ret = validate_used_block( subheap, arena, quiet ); - goto done; + else ret = validate_large_arena( heap, large_arena, quiet ); } + else ret = validate_used_block( subheap, arena, quiet ); - LIST_FOR_EACH_ENTRY( subheap, &heapPtr->subheap_list, SUBHEAP, entry ) + heap_unlock( heap, flags ); + + return ret; +} + +static BOOL heap_validate( HEAP *heap, DWORD flags, BOOL quiet ) +{ + const ARENA_LARGE *large_arena; + SUBHEAP *subheap; + BOOL ret = FALSE; + + heap_lock( heap, flags ); + + LIST_FOR_EACH_ENTRY( subheap, &heap->subheap_list, SUBHEAP, entry ) { char *ptr = (char *)subheap->base + subheap->headerSize; while (ptr < (char *)subheap->base + subheap->size) @@ -857,13 +863,14 @@ static BOOL heap_validate( HEAP *heapPtr, DWORD flags, LPCVOID block, BOOL quiet } } - LIST_FOR_EACH_ENTRY( large_arena, &heapPtr->large_list, ARENA_LARGE, entry ) - if (!validate_large_arena( heapPtr, large_arena, quiet )) goto done; + LIST_FOR_EACH_ENTRY( large_arena, &heap->large_list, ARENA_LARGE, entry ) + if (!validate_large_arena( heap, large_arena, quiet )) goto done; ret = TRUE; done: - heap_unlock( heapPtr, flags ); + heap_unlock( heap, flags ); + return ret; } @@ -883,7 +890,7 @@ static HEAP *HEAP_GetPtr( ERR("Invalid heap %p!\n", heap ); return NULL; } - if ((heapPtr->flags & HEAP_VALIDATE_ALL) && !heap_validate( heapPtr, 0, NULL, NOISY )) + if ((heapPtr->flags & HEAP_VALIDATE_ALL) && !heap_validate( heapPtr, 0, NOISY )) { if (TRACE_ON(heap)) { @@ -2013,23 +2020,22 @@ SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr ) /*********************************************************************** * RtlValidateHeap (NTDLL.@) - * - * Determine if a block is a valid allocation from a heap. - * - * PARAMS - * heap [I] Heap that block was allocated from - * flags [I] HEAP_ flags from "winnt.h" - * ptr [I] Block to check - * - * RETURNS - * Success: TRUE. The block was allocated from heap. - * Failure: FALSE, if heap is invalid or ptr was not allocated from it. */ -BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, LPCVOID ptr ) +BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, const void *ptr ) { - HEAP *heapPtr = HEAP_GetPtr( heap ); - if (!heapPtr) return FALSE; - return heap_validate( heapPtr, flags, ptr, QUIET ); + HEAP *heapPtr; + BOOLEAN ret; + + if (!(heapPtr = HEAP_GetPtr( heap ))) + ret = FALSE; + else + { + if (ptr) ret = heap_validate_ptr( heapPtr, flags, ptr, QUIET ); + else ret = heap_validate( heapPtr, flags, QUIET ); + } + + TRACE( "heap %p, flags %#x, ptr %p, return %u\n", heapPtr, flags, ptr, ret ); + return ret; } -- 2.35.1
participants (1)
-
Rémi Bernon