[tkreuzer] 63356: [NTOSKRNL] - Fix a bug in MiQueryAddressState that prevented it from returning a valid protection - Add support for PAE and x64 to MiQueryAddressState - Acquire the working set lo...
Author: tkreuzer Date: Sun May 18 16:25:40 2014 New Revision: 63356
URL: http://svn.reactos.org/svn/reactos?rev=63356&view=rev Log: [NTOSKRNL] - Fix a bug in MiQueryAddressState that prevented it from returning a valid protection - Add support for PAE and x64 to MiQueryAddressState - Acquire the working set lock in MiQueryMemoryBasicInformation before MiQueryAddressState - Fix RegionSize calculation in MiQueryMemoryBasicInformation - Handle ZeroBits and Process->VmTopDown in NtAllocateVirtualMemory - Fix a bug in calculating the ending address of a virtual allocation - Gracefully handle Vad allocation failure - Free Vad allocation on failure - Write values back to usermode only in case of success
Modified: trunk/reactos/ntoskrnl/include/internal/amd64/mm.h trunk/reactos/ntoskrnl/mm/ARM3/miarm.h trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
Modified: trunk/reactos/ntoskrnl/include/internal/amd64/mm.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/a... ============================================================================== --- trunk/reactos/ntoskrnl/include/internal/amd64/mm.h [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/include/internal/amd64/mm.h [iso-8859-1] Sun May 18 16:25:40 2014 @@ -78,6 +78,7 @@ #define MI_ZERO_PTES (32) /* FIXME - different architectures have different cache line sizes... */ #define MM_CACHE_LINE_SIZE 32 +#define MI_MAX_ZERO_BITS 53
/* Helper macros */ #define PAGE_MASK(x) ((x)&(~0xfff)) @@ -143,11 +144,13 @@
//#define TEB_BASE 0x7FFDE000
-/* On x86, these two are the same */ +/* On x64, these are the same */ #define MMPDE MMPTE #define PMMPDE PMMPTE #define MMPPE MMPTE #define PMMPPE PMMPTE +#define MMPXE MMPTE +#define PMMPXE PMMPTE #define MI_WRITE_VALID_PPE MI_WRITE_VALID_PTE
#define ValidKernelPpe ValidKernelPde
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 May 18 16:25:40 2014 @@ -52,6 +52,7 @@ #define MI_LOWEST_VAD_ADDRESS (PVOID)MM_LOWEST_USER_ADDRESS
#define MI_DEFAULT_SYSTEM_PTE_COUNT 50000 +#define MI_MAX_ZERO_BITS 21
#endif /* !_M_AMD64 */
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 May 18 16:25:40 2014 @@ -57,7 +57,7 @@ if (Vad->u.VadFlags.MemCommit == 1) { /* This is a committed VAD, so Assume the whole range is committed */ - CommittedPages = BYTES_TO_PAGES(EndingAddress - StartingAddress); + CommittedPages = (ULONG)BYTES_TO_PAGES(EndingAddress - StartingAddress);
/* Is the PDE demand-zero? */ PointerPde = MiAddressToPte(PointerPte); @@ -1309,6 +1309,12 @@
PMMPTE PointerPte, ProtoPte; PMMPDE PointerPde; +#if (_MI_PAGING_LEVELS >= 3) + PMMPPE PointerPpe; +#endif +#if (_MI_PAGING_LEVELS >= 4) + PMMPXE PointerPxe; +#endif MMPTE TempPte, TempProtoPte; BOOLEAN DemandZeroPte = TRUE, ValidPte = FALSE; ULONG State = MEM_RESERVE, Protect = 0; @@ -1321,27 +1327,70 @@ /* Get the PDE and PTE for the address */ PointerPde = MiAddressToPde(Va); PointerPte = MiAddressToPte(Va); +#if (_MI_PAGING_LEVELS >= 3) + PointerPpe = MiAddressToPpe(Va); +#endif +#if (_MI_PAGING_LEVELS >= 4) + PointerPxe = MiAddressToPxe(Va); +#endif
/* Return the next range */ *NextVa = (PVOID)((ULONG_PTR)Va + PAGE_SIZE);
- /* Is the PDE demand-zero? */ - if (PointerPde->u.Long != 0) - { - /* It is not. Is it valid? */ + do + { +#if (_MI_PAGING_LEVELS >= 4) + /* Does the PXE exist? */ + if (PointerPxe->u.Long == 0) + { + /* It does not, next range starts at the next PXE */ + *NextVa = MiPxeToAddress(PointerPxe + 1); + break; + } + + /* Is the PXE valid? */ + if (PointerPxe->u.Hard.Valid == 0) + { + /* Is isn't, fault it in (make the PPE accessible) */ + MiMakeSystemAddressValid(PointerPpe, TargetProcess); + } +#endif +#if (_MI_PAGING_LEVELS >= 3) + /* Does the PPE exist? */ + if (PointerPpe->u.Long == 0) + { + /* It does not, next range starts at the next PPE */ + *NextVa = MiPpeToAddress(PointerPpe + 1); + break; + } + + /* Is the PPE valid? */ + if (PointerPpe->u.Hard.Valid == 0) + { + /* Is isn't, fault it in (make the PDE accessible) */ + MiMakeSystemAddressValid(PointerPde, TargetProcess); + } +#endif + + /* Does the PDE exist? */ + if (PointerPde->u.Long == 0) + { + /* It does not, next range starts at the next PDE */ + *NextVa = MiPdeToAddress(PointerPde + 1); + break; + } + + /* Is the PDE valid? */ if (PointerPde->u.Hard.Valid == 0) { - /* Is isn't, fault it in */ - PointerPte = MiPteToAddress(PointerPde); + /* Is isn't, fault it in (make the PTE accessible) */ MiMakeSystemAddressValid(PointerPte, TargetProcess); - ValidPte = TRUE; - } - } - else - { - /* It is, skip it and move to the next PDE */ - *NextVa = MiPdeToAddress(PointerPde + 1); - } + } + + /* We have a PTE that we can access now! */ + ValidPte = TRUE; + + } while (FALSE);
/* Is it safe to try reading the PTE? */ if (ValidPte) @@ -1708,6 +1757,9 @@ MemoryInfo.AllocationProtect = MmProtectToValue[Vad->u.VadFlags.Protection]; MemoryInfo.Type = MEM_PRIVATE;
+ /* Acquire the working set lock (shared is enough) */ + MiLockProcessWorkingSetShared(TargetProcess, PsGetCurrentThread()); + /* Find the largest chunk of memory which has the same state and protection mask */ MemoryInfo.State = MiQueryAddressState(Address, Vad, @@ -1722,6 +1774,16 @@ if ((NewState != MemoryInfo.State) || (NewProtect != MemoryInfo.Protect)) break; Address = NextAddress; } + + /* Release the working set lock */ + MiUnlockProcessWorkingSetShared(TargetProcess, PsGetCurrentThread()); + + /* Check if we went outside of the VAD */ + if (((ULONG_PTR)Address >> PAGE_SHIFT) > Vad->EndingVpn) + { + /* Set the end of the VAD as the end address */ + Address = (PVOID)((Vad->EndingVpn + 1) << PAGE_SHIFT); + }
/* Now that we know the last VA address, calculate the region size */ MemoryInfo.RegionSize = ((ULONG_PTR)Address - (ULONG_PTR)MemoryInfo.BaseAddress); @@ -4088,11 +4150,11 @@ PEPROCESS Process; PMEMORY_AREA MemoryArea; PFN_NUMBER PageCount; - PMMVAD Vad, FoundVad; + PMMVAD Vad = NULL, FoundVad; NTSTATUS Status; PMMSUPPORT AddressSpace; PVOID PBaseAddress; - ULONG_PTR PRegionSize, StartingAddress, EndingAddress; + ULONG_PTR PRegionSize, StartingAddress, EndingAddress, HighestAddress; PEPROCESS CurrentProcess = PsGetCurrentProcess(); KPROCESSOR_MODE PreviousMode = KeGetPreviousMode(); PETHREAD CurrentThread = PsGetCurrentThread(); @@ -4106,7 +4168,7 @@ PAGED_CODE();
/* Check for valid Zero bits */ - if (ZeroBits > 21) + if (ZeroBits > MI_MAX_ZERO_BITS) { DPRINT1("Too many zero bits\n"); return STATUS_INVALID_PARAMETER_3; @@ -4114,7 +4176,7 @@
/* Check for valid Allocation Types */ if ((AllocationType & ~(MEM_COMMIT | MEM_RESERVE | MEM_RESET | MEM_PHYSICAL | - MEM_TOP_DOWN | MEM_WRITE_WATCH))) + MEM_TOP_DOWN | MEM_WRITE_WATCH | MEM_LARGE_PAGES))) { DPRINT1("Invalid Allocation Type\n"); return STATUS_INVALID_PARAMETER_5; @@ -4159,16 +4221,16 @@ return STATUS_INVALID_PARAMETER_5; }
- /* MEM_PHYSICAL can only be used if MEM_RESERVE is also used */ - if ((AllocationType & MEM_PHYSICAL) && !(AllocationType & MEM_RESERVE)) - { - DPRINT1("MEM_WRITE_WATCH used without MEM_RESERVE\n"); - return STATUS_INVALID_PARAMETER_5; - } - /* Check for valid MEM_PHYSICAL usage */ if (AllocationType & MEM_PHYSICAL) { + /* MEM_PHYSICAL can only be used if MEM_RESERVE is also used */ + if (!(AllocationType & MEM_RESERVE)) + { + DPRINT1("MEM_PHYSICAL used without MEM_RESERVE\n"); + return STATUS_INVALID_PARAMETER_5; + } + /* Only these flags are allowed with MEM_PHYSIAL */ if (AllocationType & ~(MEM_RESERVE | MEM_TOP_DOWN | MEM_PHYSICAL)) { @@ -4200,7 +4262,7 @@ { /* Make sure they are writable */ ProbeForWritePointer(UBaseAddress); - ProbeForWriteUlong(URegionSize); + ProbeForWriteSize_t(URegionSize); }
/* Capture their values */ @@ -4215,7 +4277,7 @@ _SEH2_END;
/* Make sure the allocation isn't past the VAD area */ - if (PBaseAddress >= MM_HIGHEST_VAD_ADDRESS) + if (PBaseAddress > MM_HIGHEST_VAD_ADDRESS) { DPRINT1("Virtual allocation base above User Space\n"); return STATUS_INVALID_PARAMETER_2; @@ -4280,12 +4342,6 @@ // // 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) == MEM_LARGE_PAGES) { DPRINT1("MEM_LARGE_PAGES not supported\n"); @@ -4304,18 +4360,6 @@ Status = STATUS_INVALID_PARAMETER; goto FailPathNoLock; } - if ((AllocationType & MEM_TOP_DOWN) == MEM_TOP_DOWN) - { - DPRINT1("MEM_TOP_DOWN not supported\n"); - AllocationType &= ~MEM_TOP_DOWN; - } - - 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 @@ -4326,7 +4370,7 @@ // // Do not allow COPY_ON_WRITE through this API // - if ((Protect & PAGE_WRITECOPY) || (Protect & PAGE_EXECUTE_WRITECOPY)) + if (Protect & (PAGE_WRITECOPY | PAGE_EXECUTE_WRITECOPY)) { DPRINT1("Copy on write not allowed through this path\n"); Status = STATUS_INVALID_PAGE_PROTECTION; @@ -4345,6 +4389,29 @@ PageCount = BYTES_TO_PAGES(PRegionSize); EndingAddress = 0; StartingAddress = 0; + + // + // Check if ZeroBits were specified + // + if (ZeroBits != 0) + { + // + // Calculate the highest address and check if it's valid + // + HighestAddress = MAXULONG_PTR >> ZeroBits; + if (HighestAddress > (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS) + { + Status = STATUS_INVALID_PARAMETER_3; + goto FailPathNoLock; + } + } + else + { + // + // Use the highest VAD address as maximum + // + HighestAddress = (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS; + } } else { @@ -4353,8 +4420,8 @@ // expected 64KB granularity, and see where the ending address will // fall based on the aligned address and the passed in region size // + EndingAddress = ((ULONG_PTR)PBaseAddress + PRegionSize - 1) | (PAGE_SIZE - 1); StartingAddress = ROUND_DOWN((ULONG_PTR)PBaseAddress, _64K); - EndingAddress = ((ULONG_PTR)PBaseAddress + PRegionSize - 1) | (PAGE_SIZE - 1); PageCount = BYTES_TO_PAGES(EndingAddress - StartingAddress); }
@@ -4362,7 +4429,13 @@ // Allocate and initialize the VAD // Vad = ExAllocatePoolWithTag(NonPagedPool, sizeof(MMVAD_LONG), 'SdaV'); - ASSERT(Vad != NULL); + if (Vad == NULL) + { + DPRINT1("Failed to allocate a VAD!\n"); + Status = STATUS_INSUFFICIENT_RESOURCES; + goto FailPathNoLock; + } + Vad->u.LongFlags = 0; if (AllocationType & MEM_COMMIT) Vad->u.VadFlags.MemCommit = 1; Vad->u.VadFlags.Protection = ProtectionMask; @@ -4389,11 +4462,11 @@ if (!PBaseAddress) { /* Which way should we search? */ - if (AllocationType & MEM_TOP_DOWN) + if ((AllocationType & MEM_TOP_DOWN) || Process->VmTopDown) { /* Find an address top-down */ Result = MiFindEmptyAddressRangeDownTree(PRegionSize, - (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS, + HighestAddress, _64K, &Process->VadRoot, &StartingAddress, @@ -4419,9 +4492,11 @@ // Now we know where the allocation ends. Make sure it doesn't end up // somewhere in kernel mode. // - NT_ASSERT(StartingAddress != 0); + ASSERT(StartingAddress != 0); + ASSERT(StartingAddress < (ULONG_PTR)MM_HIGHEST_USER_ADDRESS); EndingAddress = (StartingAddress + PRegionSize - 1) | (PAGE_SIZE - 1); - if ((PVOID)EndingAddress > MM_HIGHEST_VAD_ADDRESS) + ASSERT(EndingAddress > StartingAddress); + if (EndingAddress > HighestAddress) { Status = STATUS_NO_MEMORY; goto FailPath; @@ -4447,8 +4522,8 @@ // // Write out the VAD fields for this allocation // - Vad->StartingVpn = (ULONG_PTR)StartingAddress >> PAGE_SHIFT; - Vad->EndingVpn = (ULONG_PTR)EndingAddress >> PAGE_SHIFT; + Vad->StartingVpn = StartingAddress >> PAGE_SHIFT; + Vad->EndingVpn = EndingAddress >> PAGE_SHIFT;
// // FIXME: Should setup VAD bitmap @@ -4465,6 +4540,13 @@ MiUnlockProcessWorkingSetUnsafe(Process, CurrentThread);
// + // Make sure the actual region size is at least as big as the + // requested region size and update the value + // + ASSERT(PRegionSize <= (EndingAddress + 1 - StartingAddress)); + PRegionSize = (EndingAddress + 1 - StartingAddress); + + // // Update the virtual size of the process, and if this is now the highest // virtual size we have ever seen, update the peak virtual size to reflect // this. @@ -4497,6 +4579,9 @@ } _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) { + // + // Ignore exception! + // } _SEH2_END; return STATUS_SUCCESS; @@ -4814,6 +4899,14 @@ FailPath: MmUnlockAddressSpace(AddressSpace);
+ if (!NT_SUCCESS(Status)) + { + if (Vad != NULL) + { + ExFreePoolWithTag(Vad, 'SdaV'); + } + } + // // Check if we need to update the protection // @@ -4838,21 +4931,28 @@ if (ProcessHandle != NtCurrentProcess()) ObDereferenceObject(Process);
// - // Use SEH to write back the base address and the region size. In the case - // of an exception, we strangely do return back the exception code, even - // though the memory *has* been allocated. This mimics Windows behavior and - // there is not much we can do about it. - // - _SEH2_TRY - { - *URegionSize = PRegionSize; - *UBaseAddress = (PVOID)StartingAddress; - } - _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) - { - Status = _SEH2_GetExceptionCode(); - } - _SEH2_END; + // Only write back results on success + // + if (NT_SUCCESS(Status)) + { + // + // Use SEH to write back the base address and the region size. In the case + // of an exception, we strangely do return back the exception code, even + // though the memory *has* been allocated. This mimics Windows behavior and + // there is not much we can do about it. + // + _SEH2_TRY + { + *URegionSize = PRegionSize; + *UBaseAddress = (PVOID)StartingAddress; + } + _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) + { + Status = _SEH2_GetExceptionCode(); + } + _SEH2_END; + } + return Status; }
-
tkreuzer＠svn.reactos.org