[ros-diffs] [reactos] 36/100: [NTOSKRNL] Address issues raised in PR 3361 review

[Jérôme Gardou]

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

commit d2fa434cb7849442c9813ca3daac83882114cecd
Author:     Jérôme Gardou <jerome.gardou(a)reactos.org>
AuthorDate: Wed Dec 2 15:17:28 2020 +0100
Commit:     Jérôme Gardou <jerome.gardou(a)reactos.org>
CommitDate: Wed Feb 3 09:41:22 2021 +0100

    [NTOSKRNL] Address issues raised in PR 3361 review
---
 ntoskrnl/cache/section/data.c |  4 ----
 ntoskrnl/mm/ARM3/section.c    |  8 ++++++++
 ntoskrnl/mm/section.c         | 39 ++++++++++++++++-----------------------
 3 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/ntoskrnl/cache/section/data.c b/ntoskrnl/cache/section/data.c
index 1ec6a3326d9..fa83080eddb 100644
--- a/ntoskrnl/cache/section/data.c
+++ b/ntoskrnl/cache/section/data.c
@@ -274,8 +274,6 @@ MmFinalizeSegment(PMM_SECTION_SEGMENT Segment)
 
     DPRINT("Finalize segment %p\n", Segment);
 
-    __debugbreak();
-
     MmLockSectionSegment(Segment);
     RemoveEntryList(&Segment->ListOfSegments);
     if (Segment->Flags & MM_DATAFILE_SEGMENT) {
@@ -622,8 +620,6 @@ MiFreeSegmentPage(PMM_SECTION_SEGMENT Segment,
     ULONG_PTR Entry;
     PFILE_OBJECT FileObject = Segment->FileObject;
 
-    __debugbreak();
-
     Entry = MmGetPageEntrySectionSegment(Segment, FileOffset);
     DPRINTC("MiFreeSegmentPage(%p:%I64x -> Entry %Ix\n",
             Segment,
diff --git a/ntoskrnl/mm/ARM3/section.c b/ntoskrnl/mm/ARM3/section.c
index 9ea36637a22..dde3b2d8cc4 100644
--- a/ntoskrnl/mm/ARM3/section.c
+++ b/ntoskrnl/mm/ARM3/section.c
@@ -1094,6 +1094,14 @@ MiMapViewInSystemSpace(IN PVOID Section,
         *ViewSize = SectionSize - SectionOffset->QuadPart;
     }
 
+    /* Check overflow */
+    if ((SectionOffset->QuadPart + *ViewSize) < SectionOffset->QuadPart)
+    {
+        DPRINT1("Integer overflow between size & offset!\n");
+        MiDereferenceControlArea(ControlArea);
+        return STATUS_INVALID_VIEW_SIZE;
+    }
+
     /* Check if the caller wanted a larger section than the view */
     if (SectionOffset->QuadPart + *ViewSize > SectionSize)
     {
diff --git a/ntoskrnl/mm/section.c b/ntoskrnl/mm/section.c
index 6dcf764d7a1..e39f82d0f22 100644
--- a/ntoskrnl/mm/section.c
+++ b/ntoskrnl/mm/section.c
@@ -2050,7 +2050,7 @@ MmCreatePhysicalMemorySection(VOID)
     /*
      * Create the section mapping physical memory
      */
-    SectionSize.QuadPart = ~((ULONG_PTR)0);
+    SectionSize.QuadPart = MmNumberOfPhysicalPages * PAGE_SIZE;
     InitializeObjectAttributes(&Obj,
                                &Name,
                                OBJ_PERMANENT | OBJ_KERNEL_EXCLUSIVE,
@@ -2192,7 +2192,6 @@ MmCreateDataFileSection(PSECTION *SectionObject,
                             (PVOID*)&Section);
     if (!NT_SUCCESS(Status))
     {
-        ObDereferenceObject(FileObject);
         return(Status);
     }
     /*
@@ -2240,7 +2239,6 @@ MmCreateDataFileSection(PSECTION *SectionObject,
             if (MaximumSize.QuadPart == 0)
             {
                 ObDereferenceObject(Section);
-                ObDereferenceObject(FileObject);
                 return STATUS_MAPPED_FILE_SIZE_ZERO;
             }
         }
@@ -2254,7 +2252,6 @@ MmCreateDataFileSection(PSECTION *SectionObject,
             if (!NT_SUCCESS(Status))
             {
                 ObDereferenceObject(Section);
-                ObDereferenceObject(FileObject);
                 return(STATUS_SECTION_NOT_EXTENDED);
             }
         }
@@ -2263,7 +2260,6 @@ MmCreateDataFileSection(PSECTION *SectionObject,
     if (FileObject->SectionObjectPointer == NULL)
     {
         ObDereferenceObject(Section);
-        ObDereferenceObject(FileObject);
         return STATUS_INVALID_FILE_FOR_SECTION;
     }
 
@@ -2274,7 +2270,6 @@ MmCreateDataFileSection(PSECTION *SectionObject,
     if (Status != STATUS_SUCCESS)
     {
         ObDereferenceObject(Section);
-        ObDereferenceObject(FileObject);
         return(Status);
     }
 
@@ -2305,7 +2300,6 @@ MmCreateDataFileSection(PSECTION *SectionObject,
             //KeSetEvent((PVOID)&FileObject->Lock, IO_NO_INCREMENT, FALSE);
             MiReleasePfnLock(OldIrql);
             ObDereferenceObject(Section);
-            ObDereferenceObject(FileObject);
             return(STATUS_NO_MEMORY);
         }
 
@@ -2329,6 +2323,7 @@ MmCreateDataFileSection(PSECTION *SectionObject,
 
         ExInitializeFastMutex(&Segment->Lock);
         Segment->FileObject = FileObject;
+        ObReferenceObject(FileObject);
 
         Segment->Image.FileOffset = 0;
         Segment->Protection = SectionPageProtection;
@@ -2370,9 +2365,6 @@ MmCreateDataFileSection(PSECTION *SectionObject,
         }
 
         MmUnlockSectionSegment(Segment);
-
-        /* The segment already has a reference to a file object. Don't bother keeping
one.*/
-        ObDereferenceObject(FileObject);
     }
     Section->SizeOfSection = MaximumSize;
 
@@ -3053,7 +3045,6 @@ MmCreateImageSection(PSECTION *SectionObject,
                              (PVOID*)(PVOID)&Section);
     if (!NT_SUCCESS(Status))
     {
-        ObDereferenceObject(FileObject);
         return(Status);
     }
 
@@ -3093,17 +3084,14 @@ MmCreateImageSection(PSECTION *SectionObject,
         NTSTATUS StatusExeFmt;
         PMM_SECTION_SEGMENT DataSectionObject;
 
-        ImageSectionObject = ExAllocatePoolWithTag(NonPagedPool,
sizeof(MM_IMAGE_SECTION_OBJECT), TAG_MM_SECTION_SEGMENT);
+        ImageSectionObject = ExAllocatePoolZero(NonPagedPool,
sizeof(MM_IMAGE_SECTION_OBJECT), TAG_MM_SECTION_SEGMENT);
         if (ImageSectionObject == NULL)
         {
             MiReleasePfnLock(OldIrql);
-            ObDereferenceObject(FileObject);
             ObDereferenceObject(Section);
             return(STATUS_NO_MEMORY);
         }
 
-        RtlZeroMemory(ImageSectionObject, sizeof(MM_IMAGE_SECTION_OBJECT));
-
         ImageSectionObject->SegFlags = MM_SEGMENT_INCREATE;
         ImageSectionObject->RefCount = 1;
         FileObject->SectionObjectPointer->ImageSectionObject = ImageSectionObject;
@@ -3172,7 +3160,6 @@ MmCreateImageSection(PSECTION *SectionObject,
 
             ExFreePoolWithTag(ImageSectionObject, TAG_MM_SECTION_SEGMENT);
             ObDereferenceObject(Section);
-            ObDereferenceObject(FileObject);
             return(Status);
         }
 
@@ -3194,12 +3181,15 @@ MmCreateImageSection(PSECTION *SectionObject,
             ExFreePool(ImageSectionObject->Segments);
             ExFreePool(ImageSectionObject);
             ObDereferenceObject(Section);
-            ObDereferenceObject(FileObject);
             return(Status);
         }
 
         OldIrql = MiAcquirePfnLock();
         ImageSectionObject->SegFlags &= ~MM_SEGMENT_INCREATE;
+
+        /* Take a ref on the file on behalf of the newly created structure */
+        ObReferenceObject(FileObject);
+
         MiReleasePfnLock(OldIrql);
 
         Status = StatusExeFmt;
@@ -3213,9 +3203,6 @@ MmCreateImageSection(PSECTION *SectionObject,
 
         Section->Segment = (PSEGMENT)ImageSectionObject;
 
-        /* We let the Image Section Object hold the reference */
-        ObDereferenceObject(FileObject);
-
         Status = STATUS_SUCCESS;
     }
     //KeSetEvent((PVOID)&FileObject->Lock, IO_NO_INCREMENT, FALSE);
@@ -3693,8 +3680,10 @@ NtQuerySection(
             _SEH2_TRY
             {
                 *((SECTION_BASIC_INFORMATION*)SectionInformation) = Sbi;
-                if (ResultLength)
+                if (ResultLength != NULL)
+                {
                     *ResultLength = sizeof(Sbi);
+                }
             }
             _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
             {
@@ -3718,7 +3707,9 @@ NtQuerySection(
                     PSECTION_IMAGE_INFORMATION Sii =
(PSECTION_IMAGE_INFORMATION)SectionInformation;
                     *Sii = ImageSectionObject->ImageInformation;
                     if (ResultLength != NULL)
+                    {
                         *ResultLength = sizeof(*Sii);
+                    }
                 }
                 _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
                 {
@@ -4374,6 +4365,7 @@ MmCreateSection (OUT PVOID  * Section,
                                       SectionPageProtection,
                                       AllocationAttributes,
                                       FileObject);
+        ObDereferenceObject(FileObject);
     }
 #ifndef NEWCC
     else if (FileObject != NULL)
@@ -4386,6 +4378,7 @@ MmCreateSection (OUT PVOID  * Section,
                                           AllocationAttributes,
                                           FileObject,
                                           FileHandle != NULL);
+        ObDereferenceObject(FileObject);
     }
 #else
     else if (FileHandle != NULL || FileObject != NULL)
@@ -4393,8 +4386,8 @@ MmCreateSection (OUT PVOID  * Section,
         Status = MmCreateCacheSection(SectionObject,
                                       DesiredAccess,
                                       ObjectAttributes,
-                                      SizeOfSection,
-                                      InitialPageProtection,
+                                      MaximumSize,
+                                      SectionPageProtection,
                                       AllocationAttributes,
                                       FileObject);
     }