[ros-dev] patch test help needed

[Royce Mitchell III]

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)