Author: ion Date: Sun Sep 2 01:17:42 2012 New Revision: 57215
URL: http://svn.reactos.org/svn/reactos?rev=57215&view=rev Log: [NTOSKRNL]: Do not return data in failure cases in NtProtectVirtualMemory. [NTOSKRNL]: No longer support non-ARM3 sections in NtProtectVirtualMemory, as the only OS calls were already NO-OPS. [NTOSKRNL]: Always use ARM3 sections unless SEC_PHYSICAL_MEMORY is used, and make the check explicit. [NTOSKRNL]: No longer support allocating memory on top of non-ARM3 sections. [NTOSKRNL]: No longer ASSERT when certain features are not yet implemented, instead return an error code. [NTOSKRNL]: Add another check in NtFreevirtualMemory when invalid memory is being freed, insert of ASSERTing. [NTOSKRNL]: Implement and use MiIsEntireRangeCommitted when protecting memory to make sure the entire range is committed. This patch removes multiple hacks, ASSERTs, and evil mixing of ARM3 and non-ARM3 code/memory.
Modified: trunk/reactos/ntoskrnl/mm/ARM3/miarm.h trunk/reactos/ntoskrnl/mm/ARM3/section.c trunk/reactos/ntoskrnl/mm/ARM3/virtual.c trunk/reactos/ntoskrnl/mm/section.c
Modified: trunk/reactos/ntoskrnl/mm/ARM3/miarm.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/miarm.h?re... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/miarm.h [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/miarm.h [iso-8859-1] Sun Sep 2 01:17:42 2012 @@ -2009,20 +2009,6 @@
NTSTATUS NTAPI -MiRosAllocateVirtualMemory( - IN HANDLE ProcessHandle, - IN PEPROCESS Process, - IN PMEMORY_AREA MemoryArea, - IN PMMSUPPORT AddressSpace, - IN OUT PVOID* UBaseAddress, - IN BOOLEAN Attached, - IN OUT PSIZE_T URegionSize, - IN ULONG AllocationType, - IN ULONG Protect -); - -NTSTATUS -NTAPI MiRosUnmapViewInSystemSpace( IN PVOID MappedBase );
Modified: trunk/reactos/ntoskrnl/mm/ARM3/section.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/section.c?... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/section.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/section.c [iso-8859-1] Sun Sep 2 01:17:42 2012 @@ -2217,7 +2217,6 @@ /* Yep, use the entered size */ Section.SizeOfSection.QuadPart = InputMaximumSize->QuadPart; } - } else {
Modified: trunk/reactos/ntoskrnl/mm/ARM3/virtual.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/virtual.c?... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/virtual.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/virtual.c [iso-8859-1] Sun Sep 2 01:17:42 2012 @@ -1749,6 +1749,82 @@ return Status; }
+ULONG +NTAPI +MiIsEntireRangeCommitted(IN ULONG_PTR StartingAddress, + IN ULONG_PTR EndingAddress, + IN PMMVAD Vad, + IN PEPROCESS Process) +{ + PMMPTE PointerPte, LastPte, PointerPde; + BOOLEAN OnBoundary = TRUE; + PAGED_CODE(); + + /* Get the PDE and PTE addresses */ + PointerPde = MiAddressToPde(StartingAddress); + PointerPte = MiAddressToPte(StartingAddress); + LastPte = MiAddressToPte(EndingAddress); + + /* Loop all the PTEs */ + while (PointerPte <= LastPte) + { + /* Check if we've hit an new PDE boundary */ + if (OnBoundary) + { + /* Is this PDE demand zero? */ + PointerPde = MiAddressToPte(PointerPte); + if (PointerPde->u.Long != 0) + { + /* It isn't -- is it valid? */ + if (PointerPde->u.Hard.Valid == 0) + { + /* Nope, fault it in */ + PointerPte = MiPteToAddress(PointerPde); + MiMakeSystemAddressValid(PointerPte, Process); + } + } + else + { + /* The PTE was already valid, so move to the next one */ + PointerPde++; + PointerPte = MiPteToAddress(PointerPde); + + /* Is the entire VAD committed? If not, fail */ + if (!Vad->u.VadFlags.MemCommit) return FALSE; + + /* Everything is committed so far past the range, return true */ + if (PointerPte > LastPte) return TRUE; + } + } + + /* Is the PTE demand zero? */ + if (PointerPte->u.Long == 0) + { + /* Is the entire VAD committed? If not, fail */ + if (!Vad->u.VadFlags.MemCommit) return FALSE; + } + else + { + /* It isn't -- is it a decommited, invalid, or faulted PTE? */ + if ((PointerPte->u.Soft.Protection == MM_DECOMMIT) && + (PointerPte->u.Hard.Valid == 0) && + ((PointerPte->u.Soft.Prototype == 0) || + (PointerPte->u.Soft.PageFileHigh == MI_PTE_LOOKUP_NEEDED))) + { + /* Then part of the range is decommitted, so fail */ + return FALSE; + } + } + + /* Move to the next PTE */ + PointerPte++; + OnBoundary = MiIsPteOnPdeBoundary(PointerPte); + } + + /* All PTEs seem valid, and no VAD checks failed, the range is okay */ + return TRUE; +} + NTSTATUS NTAPI MiProtectVirtualMemory(IN PEPROCESS Process, @@ -1765,19 +1841,9 @@ MMPTE PteContents; //PUSHORT UsedPageTableEntries; PMMPFN Pfn1; - ULONG ProtectionMask; + ULONG ProtectionMask, OldProtect; + BOOLEAN Committed; NTSTATUS Status = STATUS_SUCCESS; - - /* Check for ROS specific memory area */ - MemoryArea = MmLocateMemoryAreaByAddress(&Process->Vm, *BaseAddress); - if ((MemoryArea) && (MemoryArea->Type == MEMORY_AREA_SECTION_VIEW)) - { - return MiRosProtectVirtualMemory(Process, - BaseAddress, - NumberOfBytesToProtect, - NewAccessProtection, - OldAccessProtection); - }
/* Calcualte base address for the VAD */ StartingAddress = (ULONG_PTR)PAGE_ALIGN((*BaseAddress)); @@ -1838,10 +1904,53 @@ goto FailPath; }
+ /* Check for ROS specific memory area */ + MemoryArea = MmLocateMemoryAreaByAddress(&Process->Vm, *BaseAddress); + if ((MemoryArea) && (MemoryArea->Type == MEMORY_AREA_SECTION_VIEW)) + { + /* Empirical evidence suggests this is only used in one critical scenario and is always a no-op */ + OldProtect = NewAccessProtection; + goto RosReturn; + } + + /* Is this section, or private memory? */ if (Vad->u.VadFlags.PrivateMemory == 0) { - /* This is a section, handled by the ROS specific code above */ - UNIMPLEMENTED; + /* Not yet supported */ + if (Vad->u.VadFlags.VadType == VadLargePageSection) + { + DPRINT1("Illegal VAD for attempting to set protection\n"); + Status = STATUS_CONFLICTING_ADDRESSES; + goto FailPath; + } + + /* Rotate VADs are not yet supported */ + if (Vad->u.VadFlags.VadType == VadRotatePhysical) + { + DPRINT1("Illegal VAD for attempting to set protection\n"); + Status = STATUS_CONFLICTING_ADDRESSES; + goto FailPath; + } + + /* Not valid on section files */ + if (NewAccessProtection & (PAGE_NOCACHE | PAGE_WRITECOMBINE)) + { + /* Fail */ + DPRINT1("Invalid protection flags for section\n"); + Status = STATUS_INVALID_PARAMETER_4; + goto FailPath; + } + + /* Check if data or page file mapping protection PTE is compatible */ + if (!Vad->ControlArea->u.Flags.Image) + { + /* Not yet */ + DPRINT1("Fixme: Not checking for valid protection\n"); + } + + /* This is a section, and this is not yet supported */ + DPRINT1("Section protection not yet supported\n"); + OldProtect = 0; } else { @@ -1849,13 +1958,26 @@ if ((NewAccessProtection & PAGE_WRITECOPY) || (NewAccessProtection & PAGE_EXECUTE_WRITECOPY)) { + DPRINT1("Invalid protection flags for private memory\n"); Status = STATUS_INVALID_PARAMETER_4; goto FailPath; }
//MiLockProcessWorkingSet(Thread, Process);
- /* TODO: Check if all pages in this range are committed */ + /* Check if all pages in this range are committed */ + Committed = MiIsEntireRangeCommitted(StartingAddress, + EndingAddress, + Vad, + Process); + if (!Committed) + { + /* Fail */ + DPRINT1("The entire range is not committed\n"); + Status = STATUS_NOT_COMMITTED; + //MiUnlockProcessWorkingSet(Thread, Process); + goto FailPath; + }
/* Compute starting and ending PTE and PDE addresses */ PointerPde = MiAddressToPde(StartingAddress); @@ -1869,13 +1991,13 @@ if (PointerPte->u.Long != 0) { /* Capture the page protection and make the PDE valid */ - *OldAccessProtection = MiGetPageProtection(PointerPte); + OldProtect = MiGetPageProtection(PointerPte); MiMakePdeExistAndMakeValid(PointerPde, Process, MM_NOIRQL); } else { /* Grab the old protection from the VAD itself */ - *OldAccessProtection = MmProtectToValue[Vad->u.VadFlags.Protection]; + OldProtect = MmProtectToValue[Vad->u.VadFlags.Protection]; }
/* Loop all the PTEs now */ @@ -1911,19 +2033,18 @@ if ((NewAccessProtection & PAGE_NOACCESS) || (NewAccessProtection & PAGE_GUARD)) { - /* TODO */ + /* The page should be in the WS and we should make it transition now */ UNIMPLEMENTED; + //continue; } - else - { - /* Write the protection mask and write it with a TLB flush */ - Pfn1->OriginalPte.u.Soft.Protection = ProtectionMask; - MiFlushTbAndCapture(Vad, - PointerPte, - ProtectionMask, - Pfn1, - TRUE); - } + + /* Write the protection mask and write it with a TLB flush */ + Pfn1->OriginalPte.u.Soft.Protection = ProtectionMask; + MiFlushTbAndCapture(Vad, + PointerPte, + ProtectionMask, + Pfn1, + TRUE); } else { @@ -1933,23 +2054,29 @@
/* The PTE is already demand-zero, just update the protection mask */ PointerPte->u.Soft.Protection = ProtectionMask; - } - + ASSERT(PointerPte->u.Long != 0); + } + + /* Move to the next PTE */ PointerPte++; }
- /* Unlock the working set and update quota charges if needed, then return */ + /* Unlock the working set */ //MiUnlockProcessWorkingSet(Thread, Process); } - -FailPath: +RosReturn: /* Unlock the address space */ MmUnlockAddressSpace(AddressSpace);
- /* Return parameters */ - *NumberOfBytesToProtect = (SIZE_T)((PUCHAR)EndingAddress - (PUCHAR)StartingAddress + 1); + /* Return parameters and success */ + *NumberOfBytesToProtect = EndingAddress - StartingAddress + 1; *BaseAddress = (PVOID)StartingAddress; - + *OldAccessProtection = OldProtect; + return STATUS_SUCCESS; + +FailPath: + /* Unlock the address space and return the failure code */ + MmUnlockAddressSpace(AddressSpace); return Status; }
@@ -3668,15 +3795,50 @@ }
// - // Assert on the things we don't yet support - // - ASSERT(ZeroBits == 0); - ASSERT((AllocationType & MEM_LARGE_PAGES) == 0); - ASSERT((AllocationType & MEM_PHYSICAL) == 0); - ASSERT((AllocationType & MEM_WRITE_WATCH) == 0); - ASSERT((AllocationType & MEM_TOP_DOWN) == 0); - ASSERT((AllocationType & MEM_RESET) == 0); - ASSERT(Process->VmTopDown == 0); + // Fail on the things we don't yet support + // + if (ZeroBits != 0) + { + DPRINT1("Zero bits not supported\n"); + Status = STATUS_INVALID_PARAMETER; + goto FailPathNoLock; + } + if ((AllocationType & MEM_LARGE_PAGES) == 1) + { + DPRINT1("MEM_LARGE_PAGES not supported\n"); + Status = STATUS_INVALID_PARAMETER; + goto FailPathNoLock; + } + if ((AllocationType & MEM_PHYSICAL) == 1) + { + DPRINT1("MEM_PHYSICAL not supported\n"); + Status = STATUS_INVALID_PARAMETER; + goto FailPathNoLock; + } + if ((AllocationType & MEM_WRITE_WATCH) == 1) + { + DPRINT1("MEM_WRITE_WATCH not supported\n"); + Status = STATUS_INVALID_PARAMETER; + goto FailPathNoLock; + } + if ((AllocationType & MEM_TOP_DOWN) == 1) + { + DPRINT1("MEM_TOP_DOWN not supported\n"); + Status = STATUS_INVALID_PARAMETER; + goto FailPathNoLock; + } + if ((AllocationType & MEM_RESET) == 1) + { + DPRINT1("MEM_RESET not supported\n"); + Status = STATUS_INVALID_PARAMETER; + goto FailPathNoLock; + } + if (Process->VmTopDown == 1) + { + DPRINT1("VmTopDown not supported\n"); + Status = STATUS_INVALID_PARAMETER; + goto FailPathNoLock; + }
// // Check if the caller is reserving memory, or committing memory and letting @@ -3895,24 +4057,10 @@ }
// - // If this is an existing section view, we call the old RosMm routine which - // has the relevant code required to handle the section scenario. In the future - // we will limit this even more so that there's almost nothing that the code - // needs to do, and it will become part of section.c in RosMm + // Make sure this is an ARM3 section // MemoryArea = MmLocateMemoryAreaByAddress(AddressSpace, (PVOID)PAGE_ROUND_DOWN(PBaseAddress)); - if (MemoryArea->Type != MEMORY_AREA_OWNED_BY_ARM3) - { - return MiRosAllocateVirtualMemory(ProcessHandle, - Process, - MemoryArea, - AddressSpace, - UBaseAddress, - Attached, - URegionSize, - AllocationType, - Protect); - } + ASSERT(MemoryArea->Type == MEMORY_AREA_OWNED_BY_ARM3);
// Is this a previously reserved section being committed? If so, enter the // special section path @@ -4324,12 +4472,21 @@ }
// - // These ASSERTs are here because ReactOS ARM3 does not currently implement - // any other kinds of VADs. - // - ASSERT(Vad->u.VadFlags.PrivateMemory == 1); + // Only private memory (except rotate VADs) can be freed through here */ + // + if ((!(Vad->u.VadFlags.PrivateMemory) && + (Vad->u.VadFlags.VadType != VadRotatePhysical)) || + (Vad->u.VadFlags.VadType == VadDevicePhysicalMemory)) + { + DPRINT1("Attempt to free section memory\n"); + Status = STATUS_UNABLE_TO_DELETE_SECTION; + goto FailPath; + } + + // + // ARM3 does not yet handle protected VM + // ASSERT(Vad->u.VadFlags.NoChange == 0); - ASSERT(Vad->u.VadFlags.VadType == VadNone);
// // Finally, make sure there is a ReactOS Mm MEMORY_AREA for this allocation @@ -4345,6 +4502,11 @@ // if (FreeType & MEM_RELEASE) { + // + // ARM3 only supports this VAD in this path + // + ASSERT(Vad->u.VadFlags.VadType == VadNone); + // // Is the caller trying to remove the whole VAD, or remove only a portion // of it? If no region size is specified, then the assumption is that the
Modified: trunk/reactos/ntoskrnl/mm/section.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/section.c?rev=5... ============================================================================== --- trunk/reactos/ntoskrnl/mm/section.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/section.c [iso-8859-1] Sun Sep 2 01:17:42 2012 @@ -2716,7 +2716,7 @@ &Obj, &SectionSize, PAGE_EXECUTE_READWRITE, - 0, + SEC_PHYSICALMEMORY, NULL, NULL); if (!NT_SUCCESS(Status)) @@ -4860,7 +4860,7 @@ PROS_SECTION_OBJECT *SectionObject = (PROS_SECTION_OBJECT *)Section;
/* Check if an ARM3 section is being created instead */ - if (!(AllocationAttributes & SEC_IMAGE) && (AllocationAttributes)) + if (!(AllocationAttributes & (SEC_IMAGE | SEC_PHYSICALMEMORY))) { if (!(FileObject) && !(FileHandle)) { @@ -4986,6 +4986,7 @@ #endif else { + ASSERT(AllocationAttributes & SEC_PHYSICALMEMORY); Status = MmCreatePageFileSection(SectionObject, DesiredAccess, ObjectAttributes, @@ -4997,133 +4998,4 @@ return Status; }
-VOID -MmModifyAttributes(IN PMMSUPPORT AddressSpace, - IN PVOID BaseAddress, - IN SIZE_T RegionSize, - IN ULONG OldType, - IN ULONG OldProtect, - IN ULONG NewType, - IN ULONG NewProtect) -{ - // - // This function is deprecated but remains in order to support VirtualAlloc - // calls with MEM_COMMIT on top of MapViewOfFile calls with SEC_RESERVE. - // - // Win32k's shared user heap, for example, uses that mechanism. The two - // conditions when this function needs to do something are ASSERTed for, - // because they should not arise. - // - if (NewType == MEM_RESERVE && OldType == MEM_COMMIT) - { - ASSERT(FALSE); - } - - if ((NewType == MEM_COMMIT) && (OldType == MEM_COMMIT)) - { - ASSERT(OldProtect == NewProtect); - } -} - -NTSTATUS -NTAPI -MiRosAllocateVirtualMemory(IN HANDLE ProcessHandle, - IN PEPROCESS Process, - IN PMEMORY_AREA MemoryArea, - IN PMMSUPPORT AddressSpace, - IN OUT PVOID* UBaseAddress, - IN BOOLEAN Attached, - IN OUT PSIZE_T URegionSize, - IN ULONG AllocationType, - IN ULONG Protect) -{ - ULONG_PTR PRegionSize; - ULONG Type, RegionSize; - NTSTATUS Status; - PVOID PBaseAddress, BaseAddress; - KAPC_STATE ApcState; - - PBaseAddress = *UBaseAddress; - PRegionSize = *URegionSize; - - BaseAddress = (PVOID)PAGE_ROUND_DOWN(PBaseAddress); - RegionSize = PAGE_ROUND_UP((ULONG_PTR)PBaseAddress + PRegionSize) - - PAGE_ROUND_DOWN(PBaseAddress); - Type = (AllocationType & MEM_COMMIT) ? MEM_COMMIT : MEM_RESERVE; - - ASSERT(PBaseAddress != 0); - ASSERT(Type == MEM_COMMIT); - ASSERT(MemoryArea->Type == MEMORY_AREA_SECTION_VIEW); - ASSERT(((ULONG_PTR)BaseAddress + RegionSize) <= (ULONG_PTR)MemoryArea->EndingAddress); - ASSERT(((ULONG_PTR)MemoryArea->EndingAddress - (ULONG_PTR)MemoryArea->StartingAddress) >= RegionSize); - ASSERT(MemoryArea->Data.SectionData.RegionListHead.Flink); - - Status = MmAlterRegion(AddressSpace, - MemoryArea->StartingAddress, - &MemoryArea->Data.SectionData.RegionListHead, - BaseAddress, - RegionSize, - Type, - Protect, - MmModifyAttributes); - - MmUnlockAddressSpace(AddressSpace); - if (Attached) KeUnstackDetachProcess(&ApcState); - if (ProcessHandle != NtCurrentProcess()) ObDereferenceObject(Process); - if (NT_SUCCESS(Status)) - { - *UBaseAddress = BaseAddress; - *URegionSize = RegionSize; - } - - return Status; -} - -NTSTATUS -NTAPI -MiRosProtectVirtualMemory(IN PEPROCESS Process, - IN OUT PVOID *BaseAddress, - IN OUT PSIZE_T NumberOfBytesToProtect, - IN ULONG NewAccessProtection, - OUT PULONG OldAccessProtection OPTIONAL) -{ - PMEMORY_AREA MemoryArea; - PMMSUPPORT AddressSpace; - ULONG OldAccessProtection_; - NTSTATUS Status; - - *NumberOfBytesToProtect = PAGE_ROUND_UP((ULONG_PTR)(*BaseAddress) + (*NumberOfBytesToProtect)) - PAGE_ROUND_DOWN(*BaseAddress); - *BaseAddress = (PVOID)PAGE_ROUND_DOWN(*BaseAddress); - - AddressSpace = &Process->Vm; - MmLockAddressSpace(AddressSpace); - MemoryArea = MmLocateMemoryAreaByAddress(AddressSpace, *BaseAddress); - if (MemoryArea == NULL || MemoryArea->DeleteInProgress) - { - MmUnlockAddressSpace(AddressSpace); - return STATUS_UNSUCCESSFUL; - } - - if (OldAccessProtection == NULL) OldAccessProtection = &OldAccessProtection_; - - if (MemoryArea->Type == MEMORY_AREA_SECTION_VIEW) - { - Status = MmProtectSectionView(AddressSpace, - MemoryArea, - *BaseAddress, - *NumberOfBytesToProtect, - NewAccessProtection, - OldAccessProtection); - } - else - { - /* FIXME: Should we return failure or success in this case? */ - Status = STATUS_CONFLICTING_ADDRESSES; - } - - MmUnlockAddressSpace(AddressSpace); - - return Status; -} - /* EOF */