[ros-diffs] [reactos] 01/01: [NTOS:MM] Fix use-after-free error

[Timo Kreuzer]

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

commit 9bc5b8357adc4bc19ffebc46238eac555ed7143a
Author:     Timo Kreuzer <timo.kreuzer(a)reactos.org>
AuthorDate: Sat Jul 29 13:48:46 2023 +0300
Commit:     Timo Kreuzer <timo.kreuzer(a)reactos.org>
CommitDate: Sat Aug 5 11:57:58 2023 +0300

    [NTOS:MM] Fix use-after-free error
    
    The VAD / memory area can get deleted when calling MmUnmapViewOfSegment, so it must
not be used after that.
---
 ntoskrnl/mm/section.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/ntoskrnl/mm/section.c b/ntoskrnl/mm/section.c
index 653b4759b27..fe07ac03df0 100644
--- a/ntoskrnl/mm/section.c
+++ b/ntoskrnl/mm/section.c
@@ -3670,13 +3670,16 @@ MiRosUnmapViewOfSection(IN PEPROCESS Process,
     {
         PMM_SECTION_SEGMENT Segment = MemoryArea->SectionData.Segment;
         PMMVAD Vad = &MemoryArea->VadNode;
+        PCONTROL_AREA ControlArea  = Vad->ControlArea;
         PFILE_OBJECT FileObject;
         SIZE_T ViewSize;
         LARGE_INTEGER ViewOffset;
         ViewOffset.QuadPart = MemoryArea->SectionData.ViewOffset;
-        
+
         InterlockedIncrement64(Segment->ReferenceCount);
 
+        ViewSize = PAGE_SIZE + ((Vad->EndingVpn - Vad->StartingVpn) <<
PAGE_SHIFT);
+
         Status = MmUnmapViewOfSegment(AddressSpace, BaseAddress);
         if (!NT_SUCCESS(Status))
         {
@@ -3685,6 +3688,10 @@ MiRosUnmapViewOfSection(IN PEPROCESS Process,
             ASSERT(NT_SUCCESS(Status));
         }
 
+        /* These might be deleted now */
+        Vad = NULL;
+        MemoryArea = NULL;
+
         if (FlagOn(*Segment->Flags, MM_PHYSICALMEMORY_SEGMENT))
         {
             /* Don't bother */
@@ -3706,11 +3713,10 @@ MiRosUnmapViewOfSection(IN PEPROCESS Process,
 
         /*
          * Flush only when last mapping is deleted.
-         * FIXME: Why Vad->ControlArea == NULL?
+         * FIXME: Why ControlArea == NULL? Or rather: is ControlArea ever not NULL here?
          */
-        if (Vad->ControlArea == NULL || Vad->ControlArea->NumberOfMappedViews ==
1)
+        if (ControlArea == NULL || ControlArea->NumberOfMappedViews == 1)
         {
-            ViewSize = PAGE_SIZE + ((Vad->EndingVpn - Vad->StartingVpn) <<
PAGE_SHIFT);
             while (ViewSize > 0)
             {
                 ULONG FlushSize = min(ViewSize, PAGE_ROUND_DOWN(MAXULONG));