https://git.reactos.org/?p=reactos.git;a=commitdiff;h=d2fa434cb7849442c9813c...
commit d2fa434cb7849442c9813ca3daac83882114cecd Author: Jérôme Gardou jerome.gardou@reactos.org AuthorDate: Wed Dec 2 15:17:28 2020 +0100 Commit: Jérôme Gardou jerome.gardou@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); }
-
Jérôme Gardou