This patch implements double-free detection in paged pool, except that for some reason ReactOS seems to just freeze solid as soon as reaching the desktop when I apply it.
Would someone please look at the patch and possibly try it to see if they get the same results, or if they see anything wrong with it?
Index: ntoskrnl/mm/ppool.c =================================================================== RCS file: /CVS/ReactOS/reactos/ntoskrnl/mm/ppool.c,v retrieving revision 1.33 diff -u -r1.33 ppool.c --- ntoskrnl/mm/ppool.c 20 Nov 2004 21:16:38 -0000 1.33 +++ ntoskrnl/mm/ppool.c 9 Dec 2004 13:49:52 -0000 @@ -22,17 +22,31 @@
#undef assert #define assert(x) if (!(x)) {DbgPrint("Assertion "#x" failed at %s:%d\n", __FILE__,__LINE__); KeBugCheck(0); } -#define ASSERT_SIZE(n) assert ( (n) <= MmPagedPoolSize && (n) >= 0 ) +#define ASSERT_SIZE(n) assert ( (n) <= MmPagedPoolSize && (n) > 0 ) #define ASSERT_PTR(p) assert ( ((size_t)(p)) >= ((size_t)MmPagedPoolBase) && ((size_t)(p)) < ((size_t)((size_t)MmPagedPoolBase+MmPagedPoolSize)) )
// to disable buffer over/under-run detection, set the following macro to 0 +#if !defined(DBG) && !defined(KDBG) +#define MM_PPOOL_REDZONE_BYTES 0 +#else #define MM_PPOOL_REDZONE_BYTES 4 -#define MM_PPOOL_REDZONE_VALUE 0xCD +#define MM_PPOOL_REDZONE_LOVALUE 0x87 +#define MM_PPOOL_REDZONE_HIVALUE 0xA5 +#define MM_PPOOL_FREEMAGIC (ULONG)(('F'<<0) + ('r'<<8) + ('E'<<16) + ('e'<<24)) +#define MM_PPOOL_USEDMAGIC (ULONG)(('u'<<0) + ('S'<<8) + ('e'<<16) + ('D'<<24)) +#define MM_PPOOL_LASTOWNER_ENTRIES 3 +#endif
typedef struct _MM_PPOOL_FREE_BLOCK_HEADER { ULONG Size; +#if MM_PPOOL_REDZONE_BYTES + ULONG FreeMagic; +#endif//MM_PPOOL_REDZONE_BYTES struct _MM_PPOOL_FREE_BLOCK_HEADER* NextFree; +#if MM_PPOOL_REDZONE_BYTES + ULONG LastOwnerStack[MM_PPOOL_LASTOWNER_ENTRIES]; +#endif//MM_PPOOL_REDZONE_BYTES } MM_PPOOL_FREE_BLOCK_HEADER, *PMM_PPOOL_FREE_BLOCK_HEADER;
@@ -40,7 +54,7 @@ { ULONG Size; #if MM_PPOOL_REDZONE_BYTES - + ULONG UsedMagic; ULONG UserSize; // how many bytes the user actually asked for... struct _MM_PPOOL_USED_BLOCK_HEADER* NextUsed; #endif//MM_PPOOL_REDZONE_BYTES @@ -86,6 +100,12 @@ MmPagedPoolFirstFreeBlock->NextFree = NULL;
#if MM_PPOOL_REDZONE_BYTES + MmPagedPoolFirstFreeBlock->FreeMagic = MM_PPOOL_FREEMAGIC; + { + int i; + for ( i = 0; i < MM_PPOOL_LASTOWNER_ENTRIES; i++ ) + MmPagedPoolFirstFreeBlock->LastOwnerStack[i] = 0; + }
MmPagedPoolFirstUsedBlock = NULL; #endif//MM_PPOOL_REDZONE_BYTES @@ -102,6 +122,9 @@ while ( p ) { DPRINT ( " 0x%x: %lu bytes (next 0x%x)\n", p, p->Size, p->NextFree ); +#if MM_PPOOL_REDZONE_BYTES + ASSERT ( p->FreeMagic == MM_PPOOL_FREEMAGIC ); +#endif//MM_PPOOL_REDZONE_BYTES ASSERT_PTR(p); ASSERT_SIZE(p->Size); count++; @@ -114,34 +137,71 @@ #define VerifyPagedPool() #endif
+#if !MM_PPOOL_REDZONE_BYTES +#define MmpRedZoneCheck(pUsed,Addr,file,line) +#else//MM_PPOOL_REDZONE_BYTES +static VOID FASTCALL +MmpRedZoneCheck ( PMM_PPOOL_USED_BLOCK_HEADER pUsed, PUCHAR Addr, const char* file, int line ) +{ + int i; + PUCHAR AddrEnd = Addr + pUsed->UserSize; + BOOL bLow = TRUE; + BOOL bHigh = TRUE; + + ASSERT_PTR(Addr); + if ( pUsed->UsedMagic == MM_PPOOL_FREEMAGIC ) + { + PMM_PPOOL_FREE_BLOCK_HEADER pFree = (PMM_PPOOL_FREE_BLOCK_HEADER)pUsed; + DPRINT1 ( "Double-free detected for Block 0x%x!\n", Addr ); + DbgPrint ( "First Free Stack Frames:" ); + for ( i = 0; i < MM_PPOOL_LASTOWNER_ENTRIES; i++ ) + DbgPrint ( " <%x>", pFree->LastOwnerStack[i] ); + KEBUGCHECK(BAD_POOL_HEADER); + } + if ( pUsed->UsedMagic != MM_PPOOL_USEDMAGIC ) + { + DPRINT1 ( "Bad magic in Block 0x%x!\n", Addr ); + KEBUGCHECK(BAD_POOL_HEADER); + } + ASSERT_SIZE(pUsed->Size); + ASSERT_SIZE(pUsed->UserSize); + ASSERT_PTR(AddrEnd); + Addr -= MM_PPOOL_REDZONE_BYTES; // this is to simplify indexing below... + for ( i = 0; i < MM_PPOOL_REDZONE_BYTES && bLow && bHigh; i++ ) + { + bLow = bLow && ( Addr[i] == MM_PPOOL_REDZONE_LOVALUE ); + bHigh = bHigh && ( AddrEnd[i] == MM_PPOOL_REDZONE_HIVALUE ); + } + if ( !bLow || !bHigh ) + { + const char* violation = "High and Low-side"; + if ( bHigh ) // high is okay, so it was just low failed + violation = "Low-side"; + else if ( bLow ) // low side is okay, so it was just high failed + violation = "High-side"; + DbgPrint("%s(%i): %s redzone violation detected for paged pool address 0x%x\n", + file, line, violation, Addr ); + DbgPrint ( "UsedMagic 0x%x, LoZone ", pUsed->UsedMagic ); + for ( i = 0; i < MM_PPOOL_REDZONE_BYTES; i++ ) + DbgPrint ( "%02x", Addr[i] ); + DbgPrint ( ", HiZone " ); + for ( i = 0; i < MM_PPOOL_REDZONE_BYTES; i++ ) + DbgPrint ( "%02x", AddrEnd[i] ); + DbgPrint ( "\n" ); + KEBUGCHECK(BAD_POOL_HEADER); + } +} +#endif//MM_PPOOL_REDZONE_BYTES + VOID STDCALL MmDbgPagedPoolRedZoneCheck ( const char* file, int line ) { #if MM_PPOOL_REDZONE_BYTES PMM_PPOOL_USED_BLOCK_HEADER pUsed = MmPagedPoolFirstUsedBlock; - int i; - BOOL bLow = TRUE; - BOOL bHigh = TRUE;
while ( pUsed ) { - PUCHAR Addr = (PUCHAR)block_to_address(pUsed); - for ( i = 0; i < MM_PPOOL_REDZONE_BYTES; i++ ) - { - bLow = bLow && ( *(Addr-i-1) == MM_PPOOL_REDZONE_VALUE ); - bHigh = bHigh && ( *(Addr+pUsed->UserSize+i) == MM_PPOOL_REDZONE_VALUE ); - } - if ( !bLow || !bHigh ) - { - const char* violation = "High and Low-side"; - if ( bHigh ) // high is okay, so it was just low failed - violation = "Low-side"; - else if ( bLow ) // low side is okay, so it was just high failed - violation = "High-side"; - DbgPrint("%s(%i): %s redzone violation detected for paged pool address 0x%x\n", - file, line, violation, Addr ); - KEBUGCHECK(0); - } + MmpRedZoneCheck ( pUsed, block_to_address(pUsed), __FILE__, __LINE__ ); pUsed = pUsed->NextUsed; } #endif//MM_PPOOL_REDZONE_BYTES @@ -270,6 +330,9 @@ (PMM_PPOOL_FREE_BLOCK_HEADER)address_to_block(BestAlignedAddr); assert ( BestAlignedAddr > Addr ); NewFreeBlock->Size = (char*)Addr + BestBlock->Size - (char*)BestAlignedAddr; +#if MM_PPOOL_REDZONE_BYTES + NewFreeBlock->FreeMagic = MM_PPOOL_FREEMAGIC; +#endif//MM_PPOOL_REDZONE_BYTES ASSERT_SIZE(NewFreeBlock->Size); BestBlock->Size = (size_t)NewFreeBlock - (size_t)Addr; ASSERT_SIZE(BestBlock->Size); @@ -344,6 +407,9 @@ NextBlock = (PMM_PPOOL_FREE_BLOCK_HEADER)((char*)BestBlock + BlockSize); //DPRINT("."); NextBlock->Size = NewSize; +#if MM_PPOOL_REDZONE_BYTES + NextBlock->FreeMagic = MM_PPOOL_FREEMAGIC; +#endif//MM_PPOOL_REDZONE_BYTES ASSERT_SIZE ( NextBlock->Size ); //DPRINT("."); NextBlock->NextFree = BestBlock->NextFree; @@ -372,6 +438,9 @@ NewBlock = (PMM_PPOOL_USED_BLOCK_HEADER)BestBlock; //DPRINT("."); NewBlock->Size = BlockSize; +#if MM_PPOOL_REDZONE_BYTES + NewBlock->UsedMagic = MM_PPOOL_USEDMAGIC; +#endif//MM_PPOOL_REDZONE_BYTES ASSERT_SIZE ( NewBlock->Size ); //DPRINT(".\n"); } @@ -397,6 +466,9 @@ */ NewBlock = (PMM_PPOOL_USED_BLOCK_HEADER)BestBlock; NewBlock->Size = NewSize; +#if MM_PPOOL_REDZONE_BYTES + NewBlock->UsedMagic = MM_PPOOL_USEDMAGIC; +#endif//MM_PPOOL_REDZONE_BYTES ASSERT_SIZE ( NewBlock->Size ); }
@@ -421,17 +493,9 @@ PUCHAR Addr = (PUCHAR)BlockAddress; //DbgPrint ( "writing buffer-overrun detection bytes" ); memset ( Addr - MM_PPOOL_REDZONE_BYTES, - MM_PPOOL_REDZONE_VALUE, MM_PPOOL_REDZONE_BYTES ); - memset ( Addr + NewBlock->UserSize, MM_PPOOL_REDZONE_VALUE, + MM_PPOOL_REDZONE_LOVALUE, MM_PPOOL_REDZONE_BYTES ); + memset ( Addr + NewBlock->UserSize, MM_PPOOL_REDZONE_HIVALUE, MM_PPOOL_REDZONE_BYTES ); - /*for ( i = 0; i < MM_PPOOL_REDZONE_BYTES; i++ ) - { - //DbgPrint("."); - *(Addr-i-1) = 0xCD; - //DbgPrint("o"); - *(Addr+NewBlock->UserSize+i) = 0xCD; - }*/ - //DbgPrint ( "done!\n" ); }
#endif//MM_PPOOL_REDZONE_BYTES @@ -452,29 +516,7 @@
ASSERT_IRQL(APC_LEVEL);
-#if MM_PPOOL_REDZONE_BYTES - // write out buffer-overrun detection bytes - { - int i; - PUCHAR Addr = (PUCHAR)Block; - //DbgPrint ( "checking buffer-overrun detection bytes..." ); - for ( i = 0; i < MM_PPOOL_REDZONE_BYTES; i++ ) - { - if (*(Addr-i-1) != MM_PPOOL_REDZONE_VALUE) - { - DPRINT1("Attempt to free memory %#08x. Redzone underrun!\n", Block); - } - if (*(Addr+UsedBlock->UserSize+i) != MM_PPOOL_REDZONE_VALUE) - { - DPRINT1("Attempt to free memory %#08x. Redzone overrun!\n", Block); - } - - assert ( *(Addr-i-1) == MM_PPOOL_REDZONE_VALUE ); - assert ( *(Addr+UsedBlock->UserSize+i) == MM_PPOOL_REDZONE_VALUE ); - } - //DbgPrint ( "done!\n" ); - } -#endif//MM_PPOOL_REDZONE_BYTES + MmpRedZoneCheck ( UsedBlock, Block, __FILE__, __LINE__ );
ExAcquireFastMutex(&MmPagedPoolLock);
@@ -503,6 +545,29 @@ * Begin setting up the newly freed block's header. */ FreeBlock->Size = UsedSize; +#if MM_PPOOL_REDZONE_BYTES + FreeBlock->FreeMagic = MM_PPOOL_FREEMAGIC; + { + PULONG Frame; + int i; +#if defined __GNUC__ + __asm__("mov %%ebp, %%ebx" : "=b" (Frame) : ); +#elif defined(_MSC_VER) + __asm mov [Frame], ebp +#endif + Frame = (PULONG)Frame[0]; // step out of ExFreePagedPool + for ( i = 0; i < MM_PPOOL_LASTOWNER_ENTRIES; i++ ) + { + if ( Frame == 0 || (ULONG)Frame == 0xDEADBEEF ) + FreeBlock->LastOwnerStack[i] = 0xDEADBEEF; + else + { + FreeBlock->LastOwnerStack[i] = Frame[1]; + Frame = (PULONG)Frame[0]; + } + } + } +#endif//MM_PPOOL_REDZONE_BYTES ASSERT_SIZE ( FreeBlock->Size );
/* @@ -533,6 +598,7 @@ /* * If the next block is immediately adjacent to the newly freed one then * merge them. + * PLEASE DO NOT WIPE OUT 'MAGIC' OR 'LASTOWNER' DATA FOR MERGED FREE BLOCKS */ if (NextBlock != NULL && ((char*)FreeBlock + FreeBlock->Size) == (char*)NextBlock) @@ -550,6 +616,7 @@ /* * If the previous block is adjacent to the newly freed one then * merge them. + * PLEASE DO NOT WIPE OUT 'MAGIC' OR 'LASTOWNER' DATA FOR MERGED FREE BLOCKS */ if (PreviousBlock != NULL && ((char*)PreviousBlock + PreviousBlock->Size) == (char*)FreeBlock)