[NTOS:MM] Do not map two pages into hyperspace in MiCopyFromUserPage. CORE-14548
authorThomas Faber <thomas.faber@reactos.org>
Sun, 15 Apr 2018 17:42:18 +0000 (19:42 +0200)
committerThomas Faber <thomas.faber@reactos.org>
Sun, 15 Apr 2018 18:07:21 +0000 (20:07 +0200)
Doing this is not only wrong because it acquires the same spinlock twice,
it also completely breaks the TLB flushing logic in MiMapPageInHyperSpace.
If the PTE with Offset 1 is still valid when a wrap-around to 0 happens,
the TLB flush on wrap-around will not clear the entry for this previous page.
After another loop around all hyperspace pages, page 1 is re-used but its
TLB entry has not been flushed, which may result into incorrect translation.

ntoskrnl/include/internal/mm.h
ntoskrnl/mm/section.c

index d5094e0..8d0234b 100644 (file)
@@ -757,8 +757,8 @@ MmAccessFault(
 NTSTATUS
 NTAPI
 MiCopyFromUserPage(
-    PFN_NUMBER NewPage,
-    PFN_NUMBER OldPage
+    PFN_NUMBER DestPage,
+    const VOID *SrcAddress
 );
 
 /* process.c *****************************************************************/
index b588f1c..e79ff5f 100644 (file)
@@ -1040,23 +1040,21 @@ BOOLEAN MiIsPageFromCache(PMEMORY_AREA MemoryArea,
 
 NTSTATUS
 NTAPI
-MiCopyFromUserPage(PFN_NUMBER DestPage, PFN_NUMBER SrcPage)
+MiCopyFromUserPage(PFN_NUMBER DestPage, const VOID *SrcAddress)
 {
     PEPROCESS Process;
-    KIRQL Irql, Irql2;
-    PVOID DestAddress, SrcAddress;
+    KIRQL Irql;
+    PVOID DestAddress;
 
     Process = PsGetCurrentProcess();
     DestAddress = MiMapPageInHyperSpace(Process, DestPage, &Irql);
-    SrcAddress = MiMapPageInHyperSpace(Process, SrcPage, &Irql2);
-    if (DestAddress == NULL || SrcAddress == NULL)
+    if (DestAddress == NULL)
     {
         return(STATUS_NO_MEMORY);
     }
     ASSERT((ULONG_PTR)DestAddress % PAGE_SIZE == 0);
     ASSERT((ULONG_PTR)SrcAddress % PAGE_SIZE == 0);
     RtlCopyMemory(DestAddress, SrcAddress, PAGE_SIZE);
-    MiUnmapPageInHyperSpace(Process, SrcAddress, Irql2);
     MiUnmapPageInHyperSpace(Process, DestAddress, Irql);
     return(STATUS_SUCCESS);
 }
@@ -1781,7 +1779,7 @@ MmAccessFaultSectionView(PMMSUPPORT AddressSpace,
     /*
      * Copy the old page
      */
-    MiCopyFromUserPage(NewPage, OldPage);
+    NT_VERIFY(NT_SUCCESS(MiCopyFromUserPage(NewPage, PAddress)));
 
     /*
      * Unshare the old page.