[ion] 57215: [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 ...
2 Sep
2012
2 Sep
'12
1:17 a.m.
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?r…
==============================================================================
--- 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=…
==============================================================================
--- 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 */
4451
days inactive
4451
days old
0 comments
1 participants
participants (1)
-
ion＠svn.reactos.org