[ros-diffs] [reactos] 65/100: [NTOS:MM] Fix a race condition when unmapping sections views

[Jérôme Gardou]

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

commit caf89b95829ca8cf851ed20103b70a645e8c5a83
Author:     Jérôme Gardou <jerome.gardou(a)reactos.org>
AuthorDate: Tue Dec 29 19:50:00 2020 +0100
Commit:     Jérôme Gardou <jerome.gardou(a)reactos.org>
CommitDate: Wed Feb 3 09:41:23 2021 +0100

    [NTOS:MM] Fix a race condition when unmapping sections views
---
 ntoskrnl/mm/ARM3/section.c | 18 ++++++++++++------
 ntoskrnl/mm/marea.c        |  6 ------
 ntoskrnl/mm/section.c      | 19 +++----------------
 3 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/ntoskrnl/mm/ARM3/section.c b/ntoskrnl/mm/ARM3/section.c
index dde3b2d8cc4..fddaf752031 100644
--- a/ntoskrnl/mm/ARM3/section.c
+++ b/ntoskrnl/mm/ARM3/section.c
@@ -833,12 +833,17 @@ MiUnmapViewOfSection(IN PEPROCESS Process,
     PEPROCESS CurrentProcess = PsGetCurrentProcess();
     PAGED_CODE();
 
+    /* Check if we need to lock the address space */
+    if (!Flags) MmLockAddressSpace(&Process->Vm);
+
     /* Check for Mm Region */
     MemoryArea = MmLocateMemoryAreaByAddress(&Process->Vm, BaseAddress);
     if ((MemoryArea) && (MemoryArea->Type != MEMORY_AREA_OWNED_BY_ARM3))
     {
         /* Call Mm API */
-        return MiRosUnmapViewOfSection(Process, BaseAddress,
Process->ProcessExiting);
+        NTSTATUS Status = MiRosUnmapViewOfSection(Process, BaseAddress,
Process->ProcessExiting);
+        if (!Flags) MmUnlockAddressSpace(&Process->Vm);
+        return Status;
     }
 
     /* Check if we should attach to the process */
@@ -849,10 +854,7 @@ MiUnmapViewOfSection(IN PEPROCESS Process,
         Attached = TRUE;
     }
 
-    /* Check if we need to lock the address space */
-    if (!Flags) MmLockAddressSpace(&Process->Vm);
-
-    /* Check if the process is already daed */
+    /* Check if the process is already dead */
     if (Process->VmDeleted)
     {
         /* Fail the call */
@@ -3116,11 +3118,15 @@ MmUnmapViewInSystemSpace(IN PVOID MappedBase)
     PAGED_CODE();
 
     /* Was this mapped by RosMm? */
+    MmLockAddressSpace(MmGetKernelAddressSpace());
     MemoryArea = MmLocateMemoryAreaByAddress(MmGetKernelAddressSpace(), MappedBase);
     if ((MemoryArea) && (MemoryArea->Type != MEMORY_AREA_OWNED_BY_ARM3))
     {
-        return MiRosUnmapViewInSystemSpace(MappedBase);
+        NTSTATUS Status = MiRosUnmapViewInSystemSpace(MappedBase);
+        MmUnlockAddressSpace(MmGetKernelAddressSpace());
+        return Status;
     }
+    MmUnlockAddressSpace(MmGetKernelAddressSpace());
 
     /* It was not, call the ARM3 routine */
     return MiUnmapViewInSystemSpace(&MmSession, MappedBase);
diff --git a/ntoskrnl/mm/marea.c b/ntoskrnl/mm/marea.c
index cd7b9594f1d..6e4226b6c35 100644
--- a/ntoskrnl/mm/marea.c
+++ b/ntoskrnl/mm/marea.c
@@ -543,9 +543,6 @@ MiRosCleanupMemoryArea(
             (Process->ActiveThreads == 1)) ||
            (Process->ActiveThreads == 0));
 
-    /* We are in cleanup, we don't need to synchronize */
-    MmUnlockAddressSpace(&Process->Vm);
-
     MemoryArea = (PMEMORY_AREA)Vad;
     BaseAddress = (PVOID)MA_GetStartingAddress(MemoryArea);
 
@@ -567,9 +564,6 @@ MiRosCleanupMemoryArea(
 
     /* Make sure this worked! */
     ASSERT(NT_SUCCESS(Status));
-
-    /* Lock the address space again */
-    MmLockAddressSpace(&Process->Vm);
 }
 
 VOID
diff --git a/ntoskrnl/mm/section.c b/ntoskrnl/mm/section.c
index 17762f8628e..1c7fbdbefc6 100644
--- a/ntoskrnl/mm/section.c
+++ b/ntoskrnl/mm/section.c
@@ -3459,6 +3459,7 @@ MmUnmapViewOfSegment(PMMSUPPORT AddressSpace,
     return(Status);
 }
 
+/* This functions must be called with a locked address space */
 NTSTATUS
 NTAPI
 MiRosUnmapViewOfSection(IN PEPROCESS Process,
@@ -3477,7 +3478,6 @@ MiRosUnmapViewOfSection(IN PEPROCESS Process,
 
     AddressSpace = Process ? &Process->Vm : MmGetKernelAddressSpace();
 
-    MmLockAddressSpace(AddressSpace);
     MemoryArea = MmLocateMemoryAreaByAddress(AddressSpace,
                  BaseAddress);
     if (MemoryArea == NULL ||
@@ -3492,7 +3492,6 @@ MiRosUnmapViewOfSection(IN PEPROCESS Process,
         if (MemoryArea) ASSERT(MemoryArea->Type != MEMORY_AREA_OWNED_BY_ARM3);
 
         DPRINT1("Unable to find memory area at address %p.\n", BaseAddress);
-        MmUnlockAddressSpace(AddressSpace);
         return STATUS_NOT_MAPPED_VIEW;
     }
 
@@ -3551,8 +3550,6 @@ MiRosUnmapViewOfSection(IN PEPROCESS Process,
         }
     }
 
-    MmUnlockAddressSpace(AddressSpace);
-
     /* Notify debugger */
     if (ImageBaseAddress && !SkipDebuggerNotify)
DbgkUnMapViewOfSection(ImageBaseAddress);
 
@@ -4248,24 +4245,14 @@ MmMapViewInSystemSpaceEx (
     return Status;
 }
 
+/* This function must be called with adress space lock held */
 NTSTATUS
 NTAPI
 MiRosUnmapViewInSystemSpace(IN PVOID MappedBase)
 {
-    PMMSUPPORT AddressSpace;
-    NTSTATUS Status;
-
     DPRINT("MmUnmapViewInSystemSpace() called\n");
 
-    AddressSpace = MmGetKernelAddressSpace();
-
-    MmLockAddressSpace(AddressSpace);
-
-    Status = MmUnmapViewOfSegment(AddressSpace, MappedBase);
-
-    MmUnlockAddressSpace(AddressSpace);
-
-    return Status;
+    return MmUnmapViewOfSegment(MmGetKernelAddressSpace(), MappedBase);
 }
 
 /**********************************************************************