01/01: [NTOS:MM] Do not map two pages into hyperspace in MiCopyFromUserPage. CORE-14548

[Thomas Faber]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b54e5c689c97cab907c77c...

commit b54e5c689c97cab907c77c47d26a8e93e38b1f9b
Author:     Thomas Faber [e-mail]
AuthorDate: Sun Apr 15 19:42:18 2018 +0200
Commit:     Thomas Faber [e-mail]
CommitDate: Sun Apr 15 20:07:21 2018 +0200

[NTOS:MM] Do not map two pages into hyperspace in MiCopyFromUserPage. CORE-14548

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 |  4 ++--
 ntoskrnl/mm/section.c          | 12 +++++-------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/ntoskrnl/include/internal/mm.h b/ntoskrnl/include/internal/mm.h
index d5094e0915..8d0234b4aa 100644
--- a/ntoskrnl/include/internal/mm.h
+++ b/ntoskrnl/include/internal/mm.h
@@ -757,8 +757,8 @@ MmAccessFault(
 NTSTATUS
 NTAPI
 MiCopyFromUserPage(
-    PFN_NUMBER NewPage,
-    PFN_NUMBER OldPage
+    PFN_NUMBER DestPage,
+    const VOID *SrcAddress
 );

/* process.c *****************************************************************/
diff --git a/ntoskrnl/mm/section.c b/ntoskrnl/mm/section.c
index b588f1c617..e79ff5f07b 100644
--- a/ntoskrnl/mm/section.c
+++ b/ntoskrnl/mm/section.c
@@ -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.