Mm bug fixes, and track page table leaks.

Fixed what I think is a bug where the Min/MaxTouched of a copied section
were being initialized before the source section lock was held. So those
boundaries could change by the time the section mappings are copied,
meaning lost pages.

Also added tracking for the number of page tables mapped on behalf of a
process.

Also fixed what I think was a page table leak on ARM caused by
PreallocatePageTables not filling in all 4 first level table slots.

include/minoca/kernel/arm.h

@@ -1061,12 +1061,16 @@ Members:
     PageDirectoryPhysical - Stores the physical address of the top level page
         directory.
 
+    PageTableCount - Stores the number of page tables (4k) allocated on
+        behalf of this process (user mode only).
+
 --*/
 
 typedef struct _ADDRESS_SPACE_ARM {
     ADDRESS_SPACE Common;
     PFIRST_LEVEL_TABLE PageDirectory;
     ULONG PageDirectoryPhysical;
+    ULONG PageTableCount;
 } ADDRESS_SPACE_ARM, *PADDRESS_SPACE_ARM;
 
 //

include/minoca/kernel/x86.h

@@ -751,12 +751,16 @@ Members:
     PageDirectoryPhysical - Stores the physical address of the top level page
         directory.
 
+    PageTableCount - Stores the number of page tables that were allocated on
+        behalf of this process (user mode only).
+
 --*/
 
 typedef struct _ADDRESS_SPACE_X86 {
     ADDRESS_SPACE Common;
     PPTE PageDirectory;
     ULONG PageDirectoryPhysical;
+    ULONG PageTableCount;
 } ADDRESS_SPACE_X86, *PADDRESS_SPACE_X86;
 
 //

kernel/mm/armv7/mapping.c

@@ -69,9 +69,9 @@ MmpDestroyPageDirectory (
 
 VOID
 MmpCreatePageTable (
+    PADDRESS_SPACE_ARM AddressSpace,
     volatile FIRST_LEVEL_TABLE *FirstLevelTable,
-    PVOID VirtualAddress,
-    BOOL CurrentProcess
+    PVOID VirtualAddress
     );
 
 VOID
@@ -1328,7 +1328,7 @@ Return Value:
     //
 
     if (FirstLevelTable[FirstIndex].Format == FLT_UNMAPPED) {
-        MmpCreatePageTable(FirstLevelTable, VirtualAddress, TRUE);
+        MmpCreatePageTable(AddressSpace, FirstLevelTable, VirtualAddress);
     }
 
     SecondLevelTable = GET_PAGE_TABLE(FirstIndex);
@@ -2097,7 +2097,7 @@ Return Value:
     //
 
     if (FirstLevelTable[FirstIndex].Format == FLT_UNMAPPED) {
-        MmpCreatePageTable(FirstLevelTable, VirtualAddress, FALSE);
+        MmpCreatePageTable(OtherAddressSpace, FirstLevelTable, VirtualAddress);
     }
 
     PageTablePhysical = (ULONG)(FirstLevelTable[FirstIndex].Entry <<
@@ -2527,11 +2527,13 @@ Return Value:
     PHYSICAL_ADDRESS Physical;
     PFIRST_LEVEL_TABLE Source;
     PADDRESS_SPACE_ARM SourceSpace;
+    ULONG Total;
 
     DestinationSpace = (PADDRESS_SPACE_ARM)DestinationAddressSpace;
     SourceSpace = (PADDRESS_SPACE_ARM)SourceAddressSpace;
     Destination = DestinationSpace->PageDirectory;
     Source = SourceSpace->PageDirectory;
+    Total = 0;
     for (Index = 0; Index < FLT_INDEX(KERNEL_VA_START); Index += 4) {
         if (Source[Index].Entry == 0) {
             continue;
@@ -2562,9 +2564,14 @@ Return Value:
             return STATUS_INSUFFICIENT_RESOURCES;
         }
 
+        Total += 1;
         Destination[Index].Entry = (ULONG)Physical >> SLT_ALIGNMENT;
+        Destination[Index + 1].Entry = (ULONG)Physical >> SLT_ALIGNMENT;
+        Destination[Index + 2].Entry = (ULONG)Physical >> SLT_ALIGNMENT;
+        Destination[Index + 3].Entry = (ULONG)Physical >> SLT_ALIGNMENT;
     }
 
+    DestinationSpace->PageTableCount = Total;
     return STATUS_SUCCESS;
 }
 
@@ -2955,6 +2962,8 @@ Return Value:
             return;
         }
 
+        AddressSpace = NULL;
+
     } else {
         AddressSpace =
               (PADDRESS_SPACE_ARM)(CurrentThread->OwningProcess->AddressSpace);
@@ -2969,9 +2978,9 @@ Return Value:
 
     while (FirstIndex <= FirstIndexEnd) {
         if (FirstLevelTable[FirstIndex].Format == FLT_UNMAPPED) {
-            MmpCreatePageTable(FirstLevelTable,
-                               (PVOID)(FirstIndex << FLT_INDEX_SHIFT),
-                               TRUE);
+            MmpCreatePageTable(AddressSpace,
+                               FirstLevelTable,
+                               (PVOID)(FirstIndex << FLT_INDEX_SHIFT));
         }
 
         FirstIndex += 1;
@@ -3158,6 +3167,7 @@ Return Value:
     PHYSICAL_ADDRESS PhysicalAddress;
     PHYSICAL_ADDRESS RunPhysicalAddress;
     UINTN RunSize;
+    ULONG Total;
 
     ASSERT(Space->PageDirectory != MmKernelFirstLevelTable);
 
@@ -3170,11 +3180,13 @@ Return Value:
     RunSize = 0;
     RunPhysicalAddress = INVALID_PHYSICAL_ADDRESS;
     FirstLevelTable = Space->PageDirectory;
+    Total = 0;
     for (FirstIndex = 0;
          FirstIndex < FLT_INDEX(KERNEL_VA_START);
          FirstIndex += 4) {
 
         if (FirstLevelTable[FirstIndex].Entry != 0) {
+            Total += 1;
             PhysicalAddress =
                    (ULONG)(FirstLevelTable[FirstIndex].Entry << SLT_ALIGNMENT);
 
@@ -3201,6 +3213,9 @@ Return Value:
         MmFreePhysicalPages(RunPhysicalAddress, RunSize >> PAGE_SHIFT);
     }
 
+    ASSERT(Total == Space->PageTableCount);
+
+    Space->PageTableCount -= Total;
     MmFreeBlock(MmPageDirectoryBlockAllocator, Space->PageDirectory);
     Space->PageDirectory = NULL;
     Space->PageDirectoryPhysical = 0;
@@ -3209,9 +3224,9 @@ Return Value:
 
 VOID
 MmpCreatePageTable (
+    PADDRESS_SPACE_ARM AddressSpace,
     volatile FIRST_LEVEL_TABLE *FirstLevelTable,
-    PVOID VirtualAddress,
-    BOOL CurrentProcess
+    PVOID VirtualAddress
     )
 
 /*++
@@ -3223,16 +3238,14 @@ Routine Description:
 
 Arguments:
 
+    AddressSpace - Supplies a pointer to the address space.
+
     FirstLevelTable - Supplies a pointer to the first level table that will own
         the page table.
 
     VirtualAddress - Supplies the virtual address that the page table will
         eventually service.
 
-    CurrentProcess - Supplies a boolean indicating if the current process is
-        requesting the page to be created (TRUE) or if the work is happening on
-        behalf of another process.
-
 Return Value:
 
     None.
@@ -3241,12 +3254,15 @@ Return Value:
 
 {
 
+    BOOL CurrentProcess;
     ULONG Entry;
     ULONG FirstIndex;
     ULONG FirstIndexDown;
     ULONG LoopIndex;
+    ULONG NewCount;
     RUNLEVEL OldRunLevel;
     PHYSICAL_ADDRESS PageTablePhysical;
+    PKPROCESS Process;
     PPROCESSOR_BLOCK ProcessorBlock;
     volatile SECOND_LEVEL_TABLE *SecondLevelTable;
     ULONG SelfMapIndex;
@@ -3266,6 +3282,17 @@ Return Value:
         return;
     }
 
+    if (VirtualAddress >= KERNEL_VA_START) {
+        CurrentProcess = TRUE;
+
+    } else {
+        CurrentProcess = FALSE;
+        Process = PsGetCurrentProcess();
+        if (Process->AddressSpace == &(AddressSpace->Common)) {
+            CurrentProcess = TRUE;
+        }
+    }
+
     //
     // Allocate a page outside of the lock. Create calls that are not just
     // synchronization calls need to be at low level.
@@ -3277,10 +3304,11 @@ Return Value:
         PageTablePhysical = (ULONG)(FirstLevelTable[FirstIndex].Entry <<
                                     SLT_ALIGNMENT);
 
-        FirstLevelTable[FirstIndex].Entry = 0;
+        NewCount = 0;
 
     } else {
         PageTablePhysical = MmpAllocatePhysicalPages(1, 0);
+        NewCount = 1;
     }
 
     ASSERT(PageTablePhysical != INVALID_PHYSICAL_ADDRESS);
@@ -3347,6 +3375,8 @@ Return Value:
         } else {
             SelfMapPageTable =
                       (PSECOND_LEVEL_TABLE)((PVOID)FirstLevelTable + FLT_SIZE);
+
+            AddressSpace->PageTableCount += NewCount;
         }
 
         SelfMapIndex = FirstIndex >> 2;

kernel/mm/imgsec.c

@@ -1315,8 +1315,6 @@ Return Value:
     INITIALIZE_LIST_HEAD(&(NewSection->ChildList));
     NewSection->AddressSpace = DestinationAddressSpace;
     NewSection->VirtualAddress = SectionToCopy->VirtualAddress;
-    NewSection->MinTouched = SectionToCopy->MinTouched;
-    NewSection->MaxTouched = SectionToCopy->MaxTouched;
     NewSection->Size = SectionToCopy->Size;
     NewSection->TruncateCount = 0;
     NewSection->SwapSpace = NULL;
@@ -1390,6 +1388,8 @@ Return Value:
     //
 
     KeAcquireQueuedLock(SectionToCopy->Lock);
+    NewSection->MinTouched = SectionToCopy->MinTouched;
+    NewSection->MaxTouched = SectionToCopy->MaxTouched;
 
     //
     // Synchronize with destruction of the parent. If the parent is on its way
@@ -2364,6 +2364,8 @@ Return Value:
     } else {
 
         ASSERT((Section->Flags & IMAGE_SECTION_SHARED) == 0);
+        ASSERT((Section->MinTouched <= VirtualAddress) &&
+               (Section->MaxTouched > VirtualAddress));
 
         MmpCopyPage(Section, VirtualAddress, PhysicalAddress);
 
@@ -3290,7 +3292,9 @@ Return Value:
     KeAcquireQueuedLock(Section->Lock);
 
     //
-    // If the section inherits from another, remove it as a child.
+    // If the section inherits from another, remove it as a child. Don't set
+    // the parent to NULL because it still may need to isolate (really hand
+    // pages over) from the parent.
     //
 
     if (Section->Parent != NULL) {

kernel/mm/x86/mapping.c

@@ -57,6 +57,7 @@ MmpDestroyPageDirectory (
 
 VOID
 MmpCreatePageTable (
+    PADDRESS_SPACE_X86 AddressSpace,
     volatile PTE *Directory,
     PVOID VirtualAddress
     );
@@ -1043,7 +1044,7 @@ Return Value:
     //
 
     if (Directory[DirectoryIndex].Present == 0) {
-        MmpCreatePageTable(Directory, VirtualAddress);
+        MmpCreatePageTable(AddressSpace, Directory, VirtualAddress);
     }
 
     ASSERT(Directory[DirectoryIndex].Present != 0);
@@ -1749,7 +1750,7 @@ Return Value:
     //
 
     if (Directory[DirectoryIndex].Present == 0) {
-        MmpCreatePageTable(Directory, VirtualAddress);
+        MmpCreatePageTable(Space, Directory, VirtualAddress);
     }
 
     PageTablePhysical = (UINTN)(Directory[DirectoryIndex].Entry << PAGE_SHIFT);
@@ -2055,11 +2056,13 @@ Return Value:
     PHYSICAL_ADDRESS Physical;
     PPTE Source;
     PADDRESS_SPACE_X86 SourceSpace;
+    ULONG Total;
 
     DestinationSpace = (PADDRESS_SPACE_X86)DestinationAddressSpace;
     SourceSpace = (PADDRESS_SPACE_X86)SourceAddressSpace;
     Destination = DestinationSpace->PageDirectory;
     Source = SourceSpace->PageDirectory;
+    Total = 0;
     for (DirectoryIndex = 0;
          DirectoryIndex < ((UINTN)KERNEL_VA_START >> PAGE_DIRECTORY_SHIFT);
          DirectoryIndex += 1) {
@@ -2094,8 +2097,10 @@ Return Value:
         }
 
         Destination[DirectoryIndex].Entry = (ULONG)Physical >> PAGE_SHIFT;
+        Total += 1;
     }
 
+    DestinationSpace->PageTableCount = Total;
     return STATUS_SUCCESS;
 }
 
@@ -2380,6 +2385,8 @@ Return Value:
             return;
         }
 
+        AddressSpace = NULL;
+
     } else {
         AddressSpace =
               (PADDRESS_SPACE_X86)(CurrentThread->OwningProcess->AddressSpace);
@@ -2395,7 +2402,8 @@ Return Value:
 
     while (DirectoryIndex <= DirectoryEndIndex) {
         if (Directory[DirectoryIndex].Present == 0) {
-            MmpCreatePageTable(Directory,
+            MmpCreatePageTable(AddressSpace,
+                               Directory,
                                (PVOID)(DirectoryIndex << PAGE_DIRECTORY_SHIFT));
         }
 
@@ -2533,6 +2541,7 @@ Return Value:
     PHYSICAL_ADDRESS PhysicalAddress;
     PHYSICAL_ADDRESS RunPhysicalAddress;
     UINTN RunSize;
+    ULONG Total;
 
     //
     // Loop through and free every allocated page table in user mode.
@@ -2540,6 +2549,7 @@ Return Value:
 
     RunSize = 0;
     RunPhysicalAddress = INVALID_PHYSICAL_ADDRESS;
+    Total = 0;
     Directory = AddressSpace->PageDirectory;
     if (Directory == NULL) {
         return;
@@ -2550,6 +2560,7 @@ Return Value:
          DirectoryIndex += 1) {
 
         if (Directory[DirectoryIndex].Entry != 0) {
+            Total += 1;
             PhysicalAddress = (ULONG)(Directory[DirectoryIndex].Entry <<
                                       PAGE_SHIFT);
 
@@ -2576,6 +2587,13 @@ Return Value:
         MmFreePhysicalPages(RunPhysicalAddress, RunSize >> PAGE_SHIFT);
     }
 
+    //
+    // Assert if page tables were leaked somewhere.
+    //
+
+    ASSERT(Total == AddressSpace->PageTableCount);
+
+    AddressSpace->PageTableCount -= Total;
     MmFreeBlock(MmPageDirectoryBlockAllocator, Directory);
     AddressSpace->PageDirectory = NULL;
     AddressSpace->PageDirectoryPhysical = INVALID_PHYSICAL_ADDRESS;
@@ -2584,6 +2602,7 @@ Return Value:
 
 VOID
 MmpCreatePageTable (
+    PADDRESS_SPACE_X86 AddressSpace,
     volatile PTE *Directory,
     PVOID VirtualAddress
     )
@@ -2597,8 +2616,10 @@ Routine Description:
 
 Arguments:
 
-    Directory - Supplies a pointer to the page directory that will own the
-        page table.
+    AddressSpace - Supplies a pointer to the address space.
+
+    Directory - Supplies a pointer to the page directory, in case this is an
+        early call and there is no address space yet.
 
     VirtualAddress - Supplies the virtual address that the page table will
         eventually service.
@@ -2612,6 +2633,7 @@ Return Value:
 {
 
     ULONG DirectoryIndex;
+    ULONG NewCount;
     PHYSICAL_ADDRESS NewPageTable;
     BOOL NewPageTableUsed;
     RUNLEVEL OldRunLevel;
@@ -2656,10 +2678,11 @@ Return Value:
         (Directory[DirectoryIndex].Entry != 0)) {
 
         NewPageTable = (ULONG)(Directory[DirectoryIndex].Entry << PAGE_SHIFT);
-        Directory[DirectoryIndex].Entry = 0;
+        NewCount = 0;
 
     } else {
         NewPageTable = MmpAllocatePhysicalPages(1, 0);
+        NewCount = 1;
     }
 
     ASSERT(NewPageTable != INVALID_PHYSICAL_ADDRESS);
@@ -2721,6 +2744,7 @@ Return Value:
 
         } else {
             Directory[DirectoryIndex].User = 1;
+            AddressSpace->PageTableCount += NewCount;
         }
 
         Directory[DirectoryIndex].Present = 1;