[2/4] ntdll: add support for the HEAP_FREE_CHECKING_ENABLED style of heap overrun detection.

Alexandre Julliard julliard at winehq.org
Tue Dec 1 13:22:18 CST 2009


Dan Kegel <dank at kegel.com> writes:

> @@ -85,6 +85,7 @@ typedef struct
>  
>  #define ARENA_INUSE_FILLER     0x55
>  #define ARENA_FREE_FILLER      0xaa
> +#define ARENA_PAD_FILLER(address) (((int)(address) & 1) ? 0xee : 0xfe)

Don't cast addresses to int.

> +static PVOID HEAP_ValidateTail( const HEAP *heap, const void *arena, const void *p, SIZE_T size, SIZE_T filler_size )
> +{
> +    unsigned char *tail = ((unsigned char *)p) + size;
> +    int j=0;

Please use correct types. j should be a SIZE_T, tail should be a const
pointer.

> @@ -1477,7 +1561,9 @@ BOOLEAN WINAPI RtlFreeHeap( HANDLE heap, ULONG flags, PVOID ptr )
>      pInUse  = (ARENA_INUSE *)ptr - 1;
>      if (!(subheap = HEAP_FindSubHeap( heapPtr, pInUse )))
>      {
> -        if (!find_large_block( heapPtr, ptr )) goto error;
> +        ARENA_LARGE *arena = find_large_block( heapPtr, ptr );
> +        if (!arena) goto error;
> +        if (!validate_large_arena( heapPtr, arena, NOISY )) goto error;

Validation should only happen when explicitly requested.

> diff --git a/dlls/ntdll/tests/heap.c b/dlls/ntdll/tests/heap.c
> new file mode 100644
> index 0000000..7cfa554
> --- /dev/null
> +++ b/dlls/ntdll/tests/heap.c

The tests should go with the existing heap tests in kernel32.

> +static void test_free_overrun(void)
> +{
> +    if ((pRtlGetNtGlobalFlags() & FLG_HEAP_ENABLE_FREE_CHECK) == 0) {
> +        skip("\
> +Skipping heap free padding overrun test.  To enable:\n\
> +on Wine, use gflags to enable global heap free checking, or set the following key:\n\
> +[HKEY_LOCAL_MACHINE\\System\\CurrentControlSet\\Control\\Session Manager]\n\
> +\"GlobalFlag\"=dword:00000020\n\
> +On Windows, use gflags to enable heap free checking for ntdll_test.exe.\n");
> +        return;

You should create your own heap with checking enabled so that it can be
tested in all cases.

Also the patch is still too large, changes to sensitive areas like the
heap code should be very small and focused.

-- 
Alexandre Julliard
julliard at winehq.org



More information about the wine-devel mailing list