From: Rémi Bernon rbernon@codeweavers.com
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 255a3850c70..7ccaf5748e2 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -1437,14 +1437,13 @@ static BOOL HEAP_ValidateInUseArena( const SUBHEAP *subheap, const ARENA_INUSE * }
-static BOOL heap_validate_ptr( HEAP *heap, const void *ptr ) +static BOOL heap_validate_ptr( HEAP *heap, const void *ptr, SUBHEAP **subheap ) { const ARENA_INUSE *arena = (const ARENA_INUSE *)ptr - 1; const ARENA_LARGE *large_arena; - SUBHEAP *subheap;
- if (!(subheap = find_subheap( heap, arena )) || - ((const char *)arena < (char *)subheap->base + subheap->headerSize)) + if (!(*subheap = find_subheap( heap, arena )) || + ((const char *)arena < (char *)(*subheap)->base + (*subheap)->headerSize)) { if (!(large_arena = find_large_block( heap, ptr ))) { @@ -1455,7 +1454,7 @@ static BOOL heap_validate_ptr( HEAP *heap, const void *ptr ) return validate_large_arena( heap, large_arena, QUIET ); }
- return HEAP_ValidateInUseArena( subheap, arena, QUIET ); + return HEAP_ValidateInUseArena( *subheap, arena, QUIET ); }
static BOOL heap_validate( HEAP *heap, BOOL quiet ) @@ -1506,24 +1505,20 @@ static BOOL validate_block_pointer( HEAP *heap, SUBHEAP **ret_subheap, const ARE SUBHEAP *subheap; BOOL ret = FALSE;
+ if (heap->flags & HEAP_VALIDATE) return heap_validate_ptr( heap, arena + 1, ret_subheap ); + if (!(*ret_subheap = subheap = find_subheap( heap, arena ))) { - ARENA_LARGE *large_arena = find_large_block( heap, arena + 1 ); - - if (!large_arena) + if (!find_large_block( heap, arena + 1 )) { 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) @@ -2102,6 +2097,7 @@ SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr ) */ BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, const void *ptr ) { + SUBHEAP *subheap; HEAP *heapPtr; BOOLEAN ret;
@@ -2110,7 +2106,7 @@ BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, const void *ptr ) else { heap_lock( heapPtr, flags ); - if (ptr) ret = heap_validate_ptr( heapPtr, ptr ); + if (ptr) ret = heap_validate_ptr( heapPtr, ptr, &subheap ); else ret = heap_validate( heapPtr, QUIET ); heap_unlock( heapPtr, flags ); }
From: Rémi Bernon rbernon@codeweavers.com
And rename it to unsafe_block_from_ptr.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 84 +++++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 47 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 7ccaf5748e2..295b215b3b2 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -930,7 +930,7 @@ static ARENA_LARGE *find_large_block( const HEAP *heap, const void *ptr ) /*********************************************************************** * validate_large_arena */ -static BOOL validate_large_arena( HEAP *heap, const ARENA_LARGE *arena, BOOL quiet ) +static BOOL validate_large_arena( const HEAP *heap, const ARENA_LARGE *arena, BOOL quiet ) { DWORD flags = heap->flags;
@@ -1437,7 +1437,7 @@ static BOOL HEAP_ValidateInUseArena( const SUBHEAP *subheap, const ARENA_INUSE * }
-static BOOL heap_validate_ptr( HEAP *heap, const void *ptr, SUBHEAP **subheap ) +static BOOL heap_validate_ptr( const HEAP *heap, const void *ptr, SUBHEAP **subheap ) { const ARENA_INUSE *arena = (const ARENA_INUSE *)ptr - 1; const ARENA_LARGE *large_arena; @@ -1494,48 +1494,43 @@ static BOOL heap_validate( HEAP *heap, BOOL quiet ) return TRUE; }
- -/*********************************************************************** - * 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 ) +static inline struct block *unsafe_block_from_ptr( const HEAP *heap, const void *ptr, SUBHEAP **subheap ) { - SUBHEAP *subheap; - BOOL ret = FALSE; + struct block *block = (struct block *)ptr - 1; + const char *err = NULL, *base, *commit_end; + SIZE_T blocks_size;
- if (heap->flags & HEAP_VALIDATE) return heap_validate_ptr( heap, arena + 1, ret_subheap ); + if (heap->flags & HEAP_VALIDATE) + { + if (!heap_validate_ptr( heap, ptr, subheap )) return NULL; + return block; + }
- if (!(*ret_subheap = subheap = find_subheap( heap, arena ))) + if ((*subheap = find_subheap( heap, block ))) { - if (!find_large_block( heap, arena + 1 )) - { - WARN( "Heap %p: pointer %p is not inside heap\n", heap, arena + 1 ); - 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 ((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; + base = subheap_base( *subheap ); + commit_end = subheap_commit_end( *subheap ); + blocks_size = (char *)last_block( *subheap ) - (char *)first_block( *subheap ); + }
- return ret; + if (!*subheap) + { + if (find_large_block( heap, ptr )) return block; + err = "block region not found"; + } + else if ((ULONG_PTR)ptr % ALIGNMENT) + err = "invalid ptr alignment"; + else if (!contains( first_block( *subheap ), blocks_size, block, sizeof(*block) )) + err = "invalid block pointer"; + else if (block_get_type( block ) == ARENA_PENDING_MAGIC || (block_get_flags( block ) & ARENA_FLAG_FREE)) + err = "already freed block"; + else if (block_get_type( block ) != ARENA_INUSE_MAGIC) + err = "invalid block header"; + else if (!contains( base, commit_end - base, block, block_get_size( block ) )) + err = "invalid block size"; + + if (err) WARN( "heap %p, block %p: %s\n", heap, block, err ); + return err ? NULL : block; }
static DWORD heap_flags_from_global_flag( DWORD flag ) @@ -1826,9 +1821,7 @@ static NTSTATUS heap_free( HEAP *heap, void *ptr ) /* Inform valgrind we are trying to free memory, so it can throw up an error message */ notify_free( ptr );
- block = (ARENA_INUSE *)ptr - 1; - if (!validate_block_pointer( heap, &subheap, block )) return STATUS_INVALID_PARAMETER; - + if (!(block = unsafe_block_from_ptr( heap, ptr, &subheap ))) return STATUS_INVALID_PARAMETER; if (!subheap) free_large_block( heap, ptr ); else HEAP_MakeInUseBlockFree( subheap, block );
@@ -1870,9 +1863,7 @@ static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size if (rounded_size < size) return STATUS_NO_MEMORY; /* overflow */ if (rounded_size < HEAP_MIN_DATA_SIZE) rounded_size = HEAP_MIN_DATA_SIZE;
- pArena = (ARENA_INUSE *)ptr - 1; - if (!validate_block_pointer( heap, &subheap, pArena )) return STATUS_INVALID_PARAMETER; - + if (!(pArena = unsafe_block_from_ptr( heap, ptr, &subheap ))) return STATUS_INVALID_PARAMETER; if (!subheap) { if (!(*ret = realloc_large_block( heap, flags, ptr, size ))) return STATUS_NO_MEMORY; @@ -2056,8 +2047,7 @@ static NTSTATUS heap_size( HEAP *heap, const void *ptr, SIZE_T *size ) const ARENA_INUSE *block; SUBHEAP *subheap;
- block = (const ARENA_INUSE *)ptr - 1; - if (!validate_block_pointer( heap, &subheap, block )) return STATUS_INVALID_PARAMETER; + if (!(block = unsafe_block_from_ptr( heap, ptr, &subheap ))) return STATUS_INVALID_PARAMETER; if (!subheap) { const ARENA_LARGE *large_arena = (const ARENA_LARGE *)ptr - 1;
From: Rémi Bernon rbernon@codeweavers.com
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 73 ++++++++++++----------------------------------- 1 file changed, 19 insertions(+), 54 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 295b215b3b2..504fd3fa743 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -926,66 +926,31 @@ static ARENA_LARGE *find_large_block( const HEAP *heap, const void *ptr ) return NULL; }
- -/*********************************************************************** - * validate_large_arena - */ -static BOOL validate_large_arena( const HEAP *heap, const ARENA_LARGE *arena, BOOL quiet ) +static BOOL validate_large_arena( const HEAP *heap, const ARENA_LARGE *arena ) { - DWORD flags = heap->flags; + const char *err = NULL;
- 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) + if ((ULONG_PTR)arena & COMMIT_MASK) + err = "invalid block alignment"; + else if (arena->size != ARENA_LARGE_SIZE || arena->magic != ARENA_LARGE_MAGIC) + err = "invalid block header"; + else if (!contains( arena, arena->block_size, arena + 1, arena->data_size )) + err = "invalid block size"; + else if (heap->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 && !err; i++) if (data[i] != ARENA_TAIL_FILLER) err = "invalid block tail"; + }
- 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; - } + if (err) + { + ERR( "heap %p, block %p: %s\n", heap, arena, err ); + if (TRACE_ON(heap)) heap_dump( heap ); } - return TRUE; -}
+ return !err; +}
/*********************************************************************** * HEAP_CreateSubHeap @@ -1451,7 +1416,7 @@ static BOOL heap_validate_ptr( const HEAP *heap, const void *ptr, SUBHEAP **subh return FALSE; }
- return validate_large_arena( heap, large_arena, QUIET ); + return validate_large_arena( heap, large_arena ); }
return HEAP_ValidateInUseArena( *subheap, arena, QUIET ); @@ -1489,7 +1454,7 @@ static BOOL heap_validate( HEAP *heap, BOOL quiet ) }
LIST_FOR_EACH_ENTRY( large_arena, &heap->large_list, ARENA_LARGE, entry ) - if (!validate_large_arena( heap, large_arena, quiet )) return FALSE; + if (!validate_large_arena( heap, large_arena )) return FALSE;
return TRUE; }
From: Rémi Bernon rbernon@codeweavers.com
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 162 +++++++++++++--------------------------------- 1 file changed, 45 insertions(+), 117 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 504fd3fa743..a4c3c8b0c9c 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -1271,140 +1271,68 @@ static BOOL HEAP_ValidateFreeArena( SUBHEAP *subheap, ARENA_FREE *pArena ) }
-/*********************************************************************** - * HEAP_ValidateInUseArena - */ -static BOOL HEAP_ValidateInUseArena( const SUBHEAP *subheap, const ARENA_INUSE *pArena, BOOL quiet ) +static BOOL validate_used_block( const SUBHEAP *subheap, const struct block *block ) { - SIZE_T size; - DWORD i, flags = subheap->heap->flags; - const char *heapEnd = (const char *)subheap->base + subheap->size; + const char *err = NULL, *base = subheap_base( subheap ), *commit_end = subheap_commit_end( subheap ); + SIZE_T blocks_size = (char *)last_block( subheap ) - (char *)first_block( subheap ); + const HEAP *heap = subheap->heap; + DWORD flags = heap->flags; + const struct block *next; + int i;
- /* Check for unaligned pointers */ - if ((ULONG_PTR)pArena % ALIGNMENT != ARENA_OFFSET) + if ((ULONG_PTR)(block + 1) % ALIGNMENT) + err = "invalid block alignment"; + else if (!contains( first_block( subheap ), blocks_size, block, sizeof(*block) )) + err = "invalid block pointer"; + else if (block_get_type( block ) != ARENA_INUSE_MAGIC && block_get_type( block ) != ARENA_PENDING_MAGIC) + err = "invalid block header"; + else if (block_get_flags( block ) & ARENA_FLAG_FREE) + err = "invalid block flags"; + else if (!contains( base, commit_end - base, block, block_get_size( block ) )) + err = "invalid block size"; + else if (block->unused_bytes > block_get_size( block ) - sizeof(*block)) + err = "invalid block unused size"; + else if ((next = next_block( subheap, block )) && (block_get_flags( next ) & ARENA_FLAG_PREV_FREE)) + err = "invalid next block flags"; + else if (block_get_flags( block ) & ARENA_FLAG_PREV_FREE) { - 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; + const struct block *prev = *((struct block **)block - 1); + if (!HEAP_IsValidArenaPtr( heap, (struct entry *)prev )) + err = "invalid previous block pointer"; + else if (!(block_get_flags( prev ) & ARENA_FLAG_FREE) || block_get_type( prev ) != ARENA_FREE_MAGIC) + err = "invalid previous block flags"; + if ((char *)prev + block_get_size( prev ) != (char *)block) + err = "invalid previous block size"; }
- /* 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) + if (!err && block_get_type( block ) == ARENA_PENDING_MAGIC) { - 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)) + const char *ptr = (char *)(block + 1), *end = (char *)block + block_get_size( block ); + while (!err && ptr < end) { - 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; + if (*(DWORD *)ptr != ARENA_FREE_FILLER) err = "free block overwritten"; + ptr += sizeof(DWORD); } } - /* Check unused size */ - if (pArena->unused_bytes > size) + else if (!err && (flags & HEAP_TAIL_CHECKING_ENABLED)) { - ERR("Heap %p: invalid unused size %08x/%08lx\n", subheap->heap, pArena->unused_bytes, size ); - return FALSE; + const unsigned char *tail = (unsigned char *)block + block_get_size( block ) - block->unused_bytes; + for (i = 0; !err && i < block->unused_bytes; i++) if (tail[i] != ARENA_TAIL_FILLER) err = "invalid block tail"; } - /* 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) + if (err) { - 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; - } + ERR( "heap %p, block %p: %s\n", heap, block, err ); + if (TRACE_ON(heap)) heap_dump( heap ); } - return TRUE; + + return !err; }
static BOOL heap_validate_ptr( const HEAP *heap, const void *ptr, SUBHEAP **subheap ) { - const ARENA_INUSE *arena = (const ARENA_INUSE *)ptr - 1; + const struct block *arena = (struct block *)ptr - 1; const ARENA_LARGE *large_arena;
if (!(*subheap = find_subheap( heap, arena )) || @@ -1419,7 +1347,7 @@ static BOOL heap_validate_ptr( const HEAP *heap, const void *ptr, SUBHEAP **subh return validate_large_arena( heap, large_arena ); }
- return HEAP_ValidateInUseArena( *subheap, arena, QUIET ); + return validate_used_block( *subheap, arena ); }
static BOOL heap_validate( HEAP *heap, BOOL quiet ) @@ -1447,7 +1375,7 @@ static BOOL heap_validate( HEAP *heap, BOOL quiet ) } else { - if (!HEAP_ValidateInUseArena( subheap, (ARENA_INUSE *)ptr, NOISY )) return FALSE; + if (!validate_used_block( subheap, (ARENA_INUSE *)ptr )) return FALSE; ptr += sizeof(ARENA_INUSE) + (*(DWORD *)ptr & ARENA_SIZE_MASK); } }
From: Rémi Bernon rbernon@codeweavers.com
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 170 +++++++++++++++------------------------------- 1 file changed, 55 insertions(+), 115 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index a4c3c8b0c9c..a23ab1a4c95 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -1143,131 +1143,71 @@ static ARENA_FREE *HEAP_FindFreeBlock( HEAP *heap, SIZE_T size, }
-/*********************************************************************** - * HEAP_IsValidArenaPtr - * - * Check that the pointer is inside the range possible for arenas. - */ -static BOOL HEAP_IsValidArenaPtr( const HEAP *heap, const ARENA_FREE *ptr ) +static BOOL is_valid_free_block( const HEAP *heap, const struct block *block ) { + const SUBHEAP *subheap; 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 = find_subheap( heap, block ))) return FALSE; + if ((char *)block >= (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; + for (i = 0; i < HEAP_NB_FREE_LISTS; i++) if (block == (struct block *)&heap->freeList[i].arena) return TRUE; return FALSE; }
- -/*********************************************************************** - * HEAP_ValidateFreeArena - */ -static BOOL HEAP_ValidateFreeArena( SUBHEAP *subheap, ARENA_FREE *pArena ) +static BOOL validate_free_block( const SUBHEAP *subheap, const struct block *block ) { - 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; - } + const char *err = NULL, *base = subheap_base( subheap ), *commit_end = subheap_commit_end( subheap ); + SIZE_T blocks_size = (char *)last_block( subheap ) - (char *)first_block( subheap ); + const struct entry *entry = (struct entry *)block; + const struct block *prev, *next; + HEAP *heap = subheap->heap; + DWORD flags = heap->flags;
- /* 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) + if ((ULONG_PTR)(block + 1) % ALIGNMENT) + err = "invalid block alignment"; + else if (!contains( first_block( subheap ), blocks_size, block, sizeof(*block) )) + err = "invalid block pointer"; + else if (block_get_type( block ) != ARENA_FREE_MAGIC) + err = "invalid block header"; + else if (!(block_get_flags( block ) & ARENA_FLAG_FREE) || (block_get_flags( block ) & ARENA_FLAG_PREV_FREE)) + err = "invalid block flags"; + else if (!contains( base, subheap_size( subheap ), block, block_get_size( block ) )) + err = "invalid block size"; + else if (!is_valid_free_block( heap, (next = (struct block *)LIST_ENTRY( entry->entry.next, struct entry, entry )) )) + err = "invalid next free block pointer"; + else if (!(block_get_flags( next ) & ARENA_FLAG_FREE) || block_get_type( next ) != ARENA_FREE_MAGIC) + err = "invalid next free block header"; + else if (!is_valid_free_block( heap, (prev = (struct block *)LIST_ENTRY( entry->entry.prev, struct entry, entry )) )) + err = "invalid previous free block pointer"; + else if (!(block_get_flags( prev ) & ARENA_FLAG_FREE) || block_get_type( prev ) != ARENA_FREE_MAGIC) + err = "invalid previous free block header"; + else if ((next = next_block( subheap, (struct block *)block ))) + { + if (!(block_get_flags( next ) & ARENA_FLAG_PREV_FREE)) + err = "invalid next block flags"; + if (*((struct block **)next - 1) != block) + err = "invalid next block back pointer"; + } + + if (!err && (flags & HEAP_FREE_CHECKING_ENABLED)) + { + const char *ptr = (char *)(entry + 1), *end = (char *)block + block_get_size( block ); + if (end > commit_end) end = commit_end; + while (!err && ptr < end) { - ERR("Heap %p: arena %p has wrong back ptr %p\n", - subheap->heap, pArena, - *((ARENA_FREE **)((char *)(pArena+1) + size) - 1)); - return FALSE; + if (*(DWORD *)ptr != ARENA_FREE_FILLER) err = "free block overwritten"; + ptr += sizeof(DWORD); } } - 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++; - } + if (err) + { + ERR( "heap %p, block %p: %s\n", heap, block, err ); + if (TRACE_ON(heap)) heap_dump( heap ); } - return TRUE; + + return !err; }
@@ -1297,7 +1237,7 @@ static BOOL validate_used_block( const SUBHEAP *subheap, const struct block *blo else if (block_get_flags( block ) & ARENA_FLAG_PREV_FREE) { const struct block *prev = *((struct block **)block - 1); - if (!HEAP_IsValidArenaPtr( heap, (struct entry *)prev )) + if (!is_valid_free_block( heap, prev )) err = "invalid previous block pointer"; else if (!(block_get_flags( prev ) & ARENA_FLAG_FREE) || block_get_type( prev ) != ARENA_FREE_MAGIC) err = "invalid previous block flags"; @@ -1353,7 +1293,7 @@ static BOOL heap_validate_ptr( const HEAP *heap, const void *ptr, SUBHEAP **subh static BOOL heap_validate( HEAP *heap, BOOL quiet ) { const ARENA_LARGE *large_arena; - SUBHEAP *subheap; + const SUBHEAP *subheap;
LIST_FOR_EACH_ENTRY( subheap, &heap->subheap_list, SUBHEAP, entry ) { @@ -1370,7 +1310,7 @@ static BOOL heap_validate( HEAP *heap, BOOL quiet ) { if (*(DWORD *)ptr & ARENA_FLAG_FREE) { - if (!HEAP_ValidateFreeArena( subheap, (ARENA_FREE *)ptr )) return FALSE; + if (!validate_free_block( subheap, (ARENA_INUSE *)ptr )) return FALSE; ptr += sizeof(ARENA_FREE) + (*(DWORD *)ptr & ARENA_SIZE_MASK); } else
-
Rémi Bernon -
Rémi Bernon (＠rbernon)