[tkreuzer] 58485: [NTOSKRNL] - Implement "!pool" kdbg extension that works like in WinDbg - Implement ExpCheckPoolAllocation to check a single allocation for sanity - Rename MEMORY_ARE::PageOpCount...

Author: tkreuzer
Date: Wed Mar 13 18:13:55 2013
New Revision: 58485

URL: http://svn.reactos.org/svn/reactos?rev=58485&view=rev
Log:
[NTOSKRNL]
- Implement "!pool" kdbg extension that works like in WinDbg
- Implement ExpCheckPoolAllocation to check a single allocation for sanity
- Rename MEMORY_ARE::PageOpCount (which is unused) to Magic and set it to 'MAre'
- Implement MiRosCheckMemoryAreas, that checks all memory areas for integrity and
PspCheckProcessList that checks the process list. The code is not used anywhere, since it
can cause a major performance impact, but it proved useful to detect non paged pool
corruptions. So I'll commit it in the hope that it will be useful.

Modified:
    trunk/reactos/ntoskrnl/include/internal/mm.h
    trunk/reactos/ntoskrnl/kdbg/kdb.h
    trunk/reactos/ntoskrnl/kdbg/kdb_cli.c
    trunk/reactos/ntoskrnl/mm/ARM3/expool.c
    trunk/reactos/ntoskrnl/mm/marea.c
    trunk/reactos/ntoskrnl/ps/kill.c

Modified: trunk/reactos/ntoskrnl/include/internal/mm.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/mm.h [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/mm.h [iso-8859-1] Wed Mar 13 18:13:55 2013
@@ -257,7 +257,7 @@
     ULONG Protect;
     ULONG Flags;
     BOOLEAN DeleteInProgress;
-    ULONG PageOpCount;
+    ULONG Magic;
     PVOID Vad;
     union
     {
@@ -586,6 +586,15 @@
                 ULONG Consumer,
                 ULONG Protection);
 
+VOID
+NTAPI
+MiRosCheckMemoryAreas(
+   PMMSUPPORT AddressSpace);
+
+VOID
+NTAPI
+MiCheckAllProcessMemoryAreas(VOID);
+
 /* npool.c *******************************************************************/
 
 VOID
@@ -1748,3 +1757,13 @@
 {
     return MmKernelAddressSpace;
 }
+
+
+/* expool.c ******************************************************************/
+
+VOID
+NTAPI
+ExpCheckPoolAllocation(
+    PVOID P,
+    POOL_TYPE PoolType,
+    ULONG Tag);

Modified: trunk/reactos/ntoskrnl/kdbg/kdb.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/kdbg/kdb.h?rev=58…
==============================================================================
--- trunk/reactos/ntoskrnl/kdbg/kdb.h [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/kdbg/kdb.h [iso-8859-1] Wed Mar 13 18:13:55 2013
@@ -114,6 +114,12 @@
    IN PCHAR Format,
    IN ...  OPTIONAL);
 
+BOOLEAN
+NTAPI
+KdbpGetHexNumber(
+    IN PCHAR pszNum,
+    OUT ULONG_PTR *pulValue);
+
 /* from kdb_expr.c */
 
 BOOLEAN

Modified: trunk/reactos/ntoskrnl/kdbg/kdb_cli.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/kdbg/kdb_cli.c?re…
==============================================================================
--- trunk/reactos/ntoskrnl/kdbg/kdb_cli.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/kdbg/kdb_cli.c [iso-8859-1] Wed Mar 13 18:13:55 2013
@@ -90,6 +90,8 @@
 static BOOLEAN KdbpCmdHelp(ULONG Argc, PCHAR Argv[]);
 static BOOLEAN KdbpCmdDmesg(ULONG Argc, PCHAR Argv[]);
 
+BOOLEAN ExpKdbgExtPool(ULONG Argc, PCHAR Argv[]);
+
 #ifdef __ROS_DWARF__
 static BOOLEAN KdbpCmdPrintStruct(ULONG Argc, PCHAR Argv[]);
 #endif
@@ -178,7 +180,8 @@
     { "set", "set [var] [value]", "Sets var to value or displays
value of var.", KdbpCmdSet },
     { "dmesg", "dmesg", "Display debug messages on screen, with
navigation on pages.", KdbpCmdDmesg },
     { "kmsg", "kmsg", "Kernel dmesg. Alias for dmesg.",
KdbpCmdDmesg },
-    { "help", "help", "Display help screen.", KdbpCmdHelp
}
+    { "help", "help", "Display help screen.", KdbpCmdHelp
},
+    { "!pool", "!pool [Address [Flags]]", "Display information
about pool allocations.", ExpKdbgExtPool }
 };
 
 /* FUNCTIONS *****************************************************************/
@@ -403,6 +406,24 @@
     return Ok;
 }
 
+BOOLEAN
+NTAPI
+KdbpGetHexNumber(
+    IN PCHAR pszNum,
+    OUT ULONG_PTR *pulValue)
+{
+    char *endptr;
+
+    /* Skip optional '0x' prefix */
+    if ((pszNum[0] == '0') && ((pszNum[1] == 'x') || (pszNum[1]
== 'X')))
+        pszNum += 2;
+
+    /* Make a number from the string (hex) */
+    *pulValue = strtoul(pszNum, &endptr, 16);
+
+    return (*endptr == '\0');
+}
+
 /*!\brief Evaluates an expression and displays the result.
  */
 static BOOLEAN

Modified: trunk/reactos/ntoskrnl/mm/ARM3/expool.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/expool.c?…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/expool.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/expool.c [iso-8859-1] Wed Mar 13 18:13:55 2013
@@ -277,6 +277,86 @@
                          (ULONG_PTR)NextEntry,
                          __LINE__,
                          (ULONG_PTR)Entry);
+        }
+    }
+}
+
+VOID
+NTAPI
+ExpCheckPoolAllocation(
+    PVOID P,
+    POOL_TYPE PoolType,
+    ULONG Tag)
+{
+    PPOOL_HEADER Entry;
+    ULONG i;
+    KIRQL OldIrql;
+    POOL_TYPE RealPoolType;
+
+    /* Get the pool header */
+    Entry = ((PPOOL_HEADER)P) - 1;
+
+    /* Check if this is a large allocation */
+    if (PAGE_ALIGN(P) == P)
+    {
+        /* Lock the pool table */
+        KeAcquireSpinLock(&ExpLargePoolTableLock, &OldIrql);
+
+        /* Find the pool tag */
+        for (i = 0; i < PoolBigPageTableSize; i++)
+        {
+            /* Check if this is our allocation */
+            if (PoolBigPageTable[i].Va == P)
+            {
+                /* Make sure the tag is ok */
+                if (PoolBigPageTable[i].Key != Tag)
+                {
+                    KeBugCheckEx(BAD_POOL_CALLER, 0x0A, (ULONG_PTR)P,
PoolBigPageTable[i].Key, Tag);
+                }
+
+                break;
+            }
+        }
+
+        /* Release the lock */
+        KeReleaseSpinLock(&ExpLargePoolTableLock, OldIrql);
+
+        if (i == PoolBigPageTableSize)
+        {
+            /* Did not find the allocation */
+            //ASSERT(FALSE);
+        }
+
+        /* Get Pool type by address */
+        RealPoolType = MmDeterminePoolType(P);
+    }
+    else
+    {
+        /* Verify the tag */
+        if (Entry->PoolTag != Tag)
+        {
+            DPRINT1("Allocation has wrong pool tag! Expected '%.4s', got
'%.4s' (0x%08lx)\n",
+                    &Tag, &Entry->PoolTag, Entry->PoolTag);
+            KeBugCheckEx(BAD_POOL_CALLER, 0x0A, (ULONG_PTR)P, Entry->PoolTag, Tag);
+        }
+
+        /* Check the rest of the header */
+        ExpCheckPoolHeader(Entry);
+
+        /* Get Pool type from entry */
+        RealPoolType = (Entry->PoolType - 1);
+    }
+
+    /* Should we check the pool type? */
+    if (PoolType != -1)
+    {
+        /* Verify the pool type */
+        if (RealPoolType != PoolType)
+        {
+            DPRINT1("Wrong pool type! Expected %s, got %s\n",
+                    PoolType & BASE_POOL_TYPE_MASK ? "PagedPool" :
"NonPagedPool",
+                    (Entry->PoolType - 1) & BASE_POOL_TYPE_MASK ?
"PagedPool" : "NonPagedPool");
+            KeBugCheckEx(BAD_POOL_CALLER, 0xCC, (ULONG_PTR)P, Entry->PoolTag, Tag);
         }
     }
 }
@@ -2436,4 +2516,101 @@
     return ExAllocatePoolWithTag(PoolType, NumberOfBytes, Tag);
 }
 
+
+BOOLEAN
+ExpKdbgExtPool(
+    ULONG Argc,
+    PCHAR Argv[])
+{
+    ULONG_PTR Address = 0, Flags = 0;
+    PVOID PoolPage;
+    PPOOL_HEADER Entry;
+    BOOLEAN ThisOne;
+    PULONG Data;
+
+    if (Argc > 1)
+    {
+        /* Get address */
+        if (!KdbpGetHexNumber(Argv[1], &Address))
+        {
+            KdbpPrint("Invalid parameter: %s\n", Argv[0]);
+            return TRUE;
+        }
+    }
+
+    if (Argc > 2)
+    {
+        /* Get address */
+        if (!KdbpGetHexNumber(Argv[1], &Flags))
+        {
+            KdbpPrint("Invalid parameter: %s\n", Argv[0]);
+            return TRUE;
+        }
+    }
+
+    /* Check if we got an address */
+    if (Address != 0)
+    {
+        /* Get the base page */
+        PoolPage = PAGE_ALIGN(Address);
+    }
+    else
+    {
+        KdbpPrint("Heap is unimplemented\n");
+        return TRUE;
+    }
+
+    /* No paging support! */
+    if (!MmIsAddressValid(PoolPage))
+    {
+        KdbpPrint("Address not accessible!\n");
+        return TRUE;
+    }
+
+    /* Get pool type */
+    if ((Address >= (ULONG_PTR)MmPagedPoolStart) && (Address <=
(ULONG_PTR)MmPagedPoolEnd))
+        KdbpPrint("Allocation is from PagedPool region\n");
+    else if ((Address >= (ULONG_PTR)MmNonPagedPoolStart) && (Address <=
(ULONG_PTR)MmNonPagedPoolEnd))
+        KdbpPrint("Allocation is from NonPagedPool region\n");
+    else
+    {
+        KdbpPrint("Address 0x%p is not within any pool!\n", (PVOID)Address);
+        return TRUE;
+    }
+
+    /* Loop all entries of that page */
+    Entry = PoolPage;
+    do
+    {
+        /* Check if the address is within that entry */
+        ThisOne = ((Address >= (ULONG_PTR)Entry) &&
+                   (Address < (ULONG_PTR)(Entry + Entry->BlockSize)));
+
+        if (!(Flags & 1) || ThisOne)
+        {
+            /* Print the line */
+            KdbpPrint("%c%p size: %4d previous size: %4d  %s  %.4s\n",
+                     ThisOne ? '*' : ' ', Entry, Entry->BlockSize,
Entry->PreviousSize,
+                     (Flags & 0x80000000) ? "" : (Entry->PoolType ?
"(Allocated)" : "(Free)     "),
+                     (Flags & 0x80000000) ? "" :
(PCHAR)&Entry->PoolTag);
+        }
+
+        if (Flags & 1)
+        {
+            Data = (PULONG)(Entry + 1);
+            KdbpPrint("    %p  %08lx %08lx %08lx %08lx\n"
+                     "    %p  %08lx %08lx %08lx %08lx\n",
+                     &Data[0], Data[0], Data[1], Data[2], Data[3],
+                     &Data[4], Data[4], Data[5], Data[6], Data[7]);
+        }
+
+        /* Go to next entry */
+        Entry = POOL_BLOCK(Entry, Entry->BlockSize);
+    }
+    while ((Entry->BlockSize != 0) && ((ULONG_PTR)Entry <
(ULONG_PTR)PoolPage + PAGE_SIZE));
+
+    return TRUE;
+}
+
+
 /* EOF */

Modified: trunk/reactos/ntoskrnl/mm/marea.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/marea.c?rev=58…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/marea.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/marea.c [iso-8859-1] Wed Mar 13 18:13:55 2013
@@ -401,7 +401,7 @@
        Vad->u.VadFlags.Spare = 1;
        Vad->u.VadFlags.PrivateMemory = 1;
        Vad->u.VadFlags.Protection = MiMakeProtectionMask(marea->Protect);
-       
+
        /* Insert the VAD */
        MiInsertVad(Vad, Process);
        marea->Vad = Vad;
@@ -675,6 +675,100 @@
 NTAPI
 MiRemoveNode(IN PMMADDRESS_NODE Node,
 IN PMM_AVL_TABLE Table);
+
+#if DBG
+
+static
+VOID
+MiRosCheckMemoryAreasRecursive(
+   PMEMORY_AREA Node)
+{
+    /* Check if the allocation is ok */
+    ExpCheckPoolAllocation(Node, NonPagedPool, 'ERAM');
+
+    /* Check some fields */
+    ASSERT(Node->Magic == 'erAM');
+    ASSERT(PAGE_ALIGN(Node->StartingAddress) == Node->StartingAddress);
+    ASSERT(Node->EndingAddress != NULL);
+    ASSERT(PAGE_ALIGN(Node->EndingAddress) == Node->EndingAddress);
+    ASSERT((ULONG_PTR)Node->StartingAddress < (ULONG_PTR)Node->EndingAddress);
+    ASSERT((Node->Type == 0) ||
+           (Node->Type == MEMORY_AREA_CACHE) ||
+          // (Node->Type == MEMORY_AREA_CACHE_SEGMENT) ||
+           (Node->Type == MEMORY_AREA_SECTION_VIEW) ||
+           (Node->Type == MEMORY_AREA_OWNED_BY_ARM3) ||
+           (Node->Type == (MEMORY_AREA_OWNED_BY_ARM3 | MEMORY_AREA_STATIC)));
+
+    /* Recursively check children */
+    if (Node->LeftChild != NULL)
+        MiRosCheckMemoryAreasRecursive(Node->LeftChild);
+    if (Node->RightChild != NULL)
+        MiRosCheckMemoryAreasRecursive(Node->RightChild);
+}
+
+VOID
+NTAPI
+MiRosCheckMemoryAreas(
+   PMMSUPPORT AddressSpace)
+{
+    PMEMORY_AREA RootNode;
+    PEPROCESS AddressSpaceOwner;
+    BOOLEAN NeedReleaseLock;
+
+    NeedReleaseLock = FALSE;
+
+    /* Get the address space owner */
+    AddressSpaceOwner = CONTAINING_RECORD(AddressSpace, EPROCESS, Vm);
+
+    /* Check if we already own the address space lock */
+    if (AddressSpaceOwner->AddressCreationLock.Owner != KeGetCurrentThread())
+    {
+        /* We must own it! */
+        MmLockAddressSpace(AddressSpace);
+        NeedReleaseLock = TRUE;
+    }
+
+    /* Check all memory areas */
+    RootNode = (PMEMORY_AREA)AddressSpace->WorkingSetExpansionLinks.Flink;
+    MiRosCheckMemoryAreasRecursive(RootNode);
+
+    /* Release the lock, if we acquired it */
+    if (NeedReleaseLock)
+    {
+        MmUnlockAddressSpace(AddressSpace);
+    }
+}
+
+extern KGUARDED_MUTEX PspActiveProcessMutex;
+
+VOID
+NTAPI
+MiCheckAllProcessMemoryAreas(VOID)
+{
+    PEPROCESS Process;
+    PLIST_ENTRY Entry;
+
+    /* Acquire the Active Process Lock */
+    KeAcquireGuardedMutex(&PspActiveProcessMutex);
+
+    /* Loop the process list */
+    Entry = PsActiveProcessHead.Flink;
+    while (Entry != &PsActiveProcessHead)
+    {
+        /* Get the process */
+        Process = CONTAINING_RECORD(Entry, EPROCESS, ActiveProcessLinks);
+
+        /* Check memory areas */
+        MiRosCheckMemoryAreas(&Process->Vm);
+
+        Entry = Entry->Flink;
+    }
+
+    /* Release the lock */
+    KeReleaseGuardedMutex(&PspActiveProcessMutex);
+}
+
+#endif
 
 /**
  * @name MmFreeMemoryArea
@@ -712,6 +806,12 @@
    ULONG_PTR Address;
    PVOID EndAddress;
 
+   /* Make sure we own the address space lock! */
+   ASSERT(CONTAINING_RECORD(AddressSpace, EPROCESS, Vm)->AddressCreationLock.Owner ==
KeGetCurrentThread());
+
+    /* Check magic */
+    ASSERT(MemoryArea->Magic == 'erAM');
+
    if (MemoryArea->Type != MEMORY_AREA_OWNED_BY_ARM3)
    {
        PEPROCESS CurrentProcess = PsGetCurrentProcess();
@@ -731,7 +831,7 @@
             BOOLEAN Dirty = FALSE;
             SWAPENTRY SwapEntry = 0;
             PFN_NUMBER Page = 0;
-             
+
              if (MmIsPageSwapEntry(Process, (PVOID)Address))
              {
                 MmDeletePageFileMapping(Process, (PVOID)Address, &SwapEntry);
@@ -788,9 +888,6 @@
        }
     }
 
-    /* There must be no page ops in progress */
-    ASSERT(MemoryArea->PageOpCount == 0);
-
    /* Remove the tree item. */
    {
       if (MemoryArea->Parent != NULL)
@@ -979,7 +1076,7 @@
    MemoryArea->Protect = Protect;
    MemoryArea->Flags = AllocationFlags;
    //MemoryArea->LockCount = 0;
-   MemoryArea->PageOpCount = 0;
+   MemoryArea->Magic = 'erAM';
    MemoryArea->DeleteInProgress = FALSE;
 
    MmInsertMemoryArea(AddressSpace, MemoryArea);
@@ -1072,17 +1169,17 @@
             KeBugCheck(MEMORY_MANAGEMENT);
       }
    }
-   
+
 #if (_MI_PAGING_LEVELS == 2)
     {
         KIRQL OldIrql;
         PMMPDE pointerPde;
         /* Attach to Process */
         KeAttachProcess(&Process->Pcb);
-        
+
         /* Acquire PFN lock */
         OldIrql = KeAcquireQueuedSpinLock(LockQueuePfnLock);
-        
+
         for(Address = MI_LOWEST_VAD_ADDRESS;
             Address < MM_HIGHEST_VAD_ADDRESS;
             Address =(PVOID)((ULONG_PTR)Address + (PAGE_SIZE * PTE_COUNT)))
@@ -1098,7 +1195,7 @@
         }
         /* Release lock */
         KeReleaseQueuedSpinLock(LockQueuePfnLock, OldIrql);
-        
+
         /* Detach */
         KeDetachProcess();
     }

Modified: trunk/reactos/ntoskrnl/ps/kill.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ps/kill.c?rev=584…
==============================================================================
--- trunk/reactos/ntoskrnl/ps/kill.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ps/kill.c [iso-8859-1] Wed Mar 13 18:13:55 2013
@@ -205,6 +205,52 @@
                                                (PVOID)1) != (PVOID)1);
 }
 
+#if DBG
+VOID
+NTAPI
+PspCheckProcessList()
+{
+    PLIST_ENTRY Entry;
+
+    KeAcquireGuardedMutex(&PspActiveProcessMutex);
+    DbgPrint("# checking PsActiveProcessHead @ %p\n",
&PsActiveProcessHead);
+    for (Entry = PsActiveProcessHead.Flink;
+         Entry != &PsActiveProcessHead;
+         Entry = Entry->Flink)
+    {
+        PEPROCESS Process = CONTAINING_RECORD(Entry, EPROCESS, ActiveProcessLinks);
+        POBJECT_HEADER Header;
+        PVOID Info, HeaderLocation;
+
+        /* Get the header and assume this is what we'll free */
+        Header = OBJECT_TO_OBJECT_HEADER(Process);
+        HeaderLocation = Header;
+
+        /* To find the header, walk backwards from how we allocated */
+        if ((Info = OBJECT_HEADER_TO_CREATOR_INFO(Header)))
+        {
+            HeaderLocation = Info;
+        }
+        if ((Info = OBJECT_HEADER_TO_NAME_INFO(Header)))
+        {
+            HeaderLocation = Info;
+        }
+        if ((Info = OBJECT_HEADER_TO_HANDLE_INFO(Header)))
+        {
+            HeaderLocation = Info;
+        }
+        if ((Info = OBJECT_HEADER_TO_QUOTA_INFO(Header)))
+        {
+            HeaderLocation = Info;
+        }
+
+        ExpCheckPoolAllocation(HeaderLocation, NonPagedPool, 'corP');
+    }
+
+    KeReleaseGuardedMutex(&PspActiveProcessMutex);
+}
+#endif
+
 VOID
 NTAPI
 PspDeleteProcess(IN PVOID ObjectBody)
@@ -221,6 +267,8 @@
         /* Remove it from the Active List */
         KeAcquireGuardedMutex(&PspActiveProcessMutex);
         RemoveEntryList(&Process->ActiveProcessLinks);
+        Process->ActiveProcessLinks.Flink = NULL;
+        Process->ActiveProcessLinks.Blink = NULL;
         KeReleaseGuardedMutex(&PspActiveProcessMutex);
     }