From 8d02368e52d8188939939431512cc7cc7008a01e Mon Sep 17 00:00:00 2001 From: Timo Kreuzer Date: Sun, 18 May 2014 16:25:40 +0000 Subject: [PATCH] [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 svn path=/trunk/; revision=63356 --- reactos/ntoskrnl/include/internal/amd64/mm.h | 5 +- reactos/ntoskrnl/mm/ARM3/miarm.h | 1 + reactos/ntoskrnl/mm/ARM3/virtual.c | 230 +++++++++++++------ 3 files changed, 170 insertions(+), 66 deletions(-) diff --git a/reactos/ntoskrnl/include/internal/amd64/mm.h b/reactos/ntoskrnl/include/internal/amd64/mm.h index 09049141e51..661955c4e08 100644 --- a/reactos/ntoskrnl/include/internal/amd64/mm.h +++ b/reactos/ntoskrnl/include/internal/amd64/mm.h @@ -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 diff --git a/reactos/ntoskrnl/mm/ARM3/miarm.h b/reactos/ntoskrnl/mm/ARM3/miarm.h index 1a86f7e40fc..d1cfbb4a24d 100644 --- a/reactos/ntoskrnl/mm/ARM3/miarm.h +++ b/reactos/ntoskrnl/mm/ARM3/miarm.h @@ -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 */ diff --git a/reactos/ntoskrnl/mm/ARM3/virtual.c b/reactos/ntoskrnl/mm/ARM3/virtual.c index ea2f09405a4..935fc971dda 100644 --- a/reactos/ntoskrnl/mm/ARM3/virtual.c +++ b/reactos/ntoskrnl/mm/ARM3/virtual.c @@ -57,7 +57,7 @@ MiCalculatePageCommitment(IN ULONG_PTR StartingAddress, 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 @@ MiQueryAddressState(IN PVOID Va, 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 @@ MiQueryAddressState(IN PVOID Va, /* 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) + do { - /* It is not. Is it valid? */ +#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 @@ MiQueryMemoryBasicInformation(IN HANDLE ProcessHandle, 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, @@ -1723,6 +1775,16 @@ MiQueryMemoryBasicInformation(IN HANDLE ProcessHandle, 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, /* 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, { /* Make sure they are writable */ ProbeForWritePointer(UBaseAddress); - ProbeForWriteUlong(URegionSize); + ProbeForWriteSize_t(URegionSize); } /* Capture their values */ @@ -4215,7 +4277,7 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, _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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, // // 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, // // 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, // expected 64KB granularity, and see where the ending address will // fall based on the aligned address and the passed in region size // - StartingAddress = ROUND_DOWN((ULONG_PTR)PBaseAddress, _64K); EndingAddress = ((ULONG_PTR)PBaseAddress + PRegionSize - 1) | (PAGE_SIZE - 1); + StartingAddress = ROUND_DOWN((ULONG_PTR)PBaseAddress, _64K); PageCount = BYTES_TO_PAGES(EndingAddress - StartingAddress); } @@ -4362,7 +4429,13 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, // 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, // 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 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, // // 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 @@ -4464,6 +4539,13 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, MiInsertNode(&Process->VadRoot, (PVOID)Vad, Parent, Result); 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 @@ -4497,6 +4579,9 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, } _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) { + // + // Ignore exception! + // } _SEH2_END; return STATUS_SUCCESS; @@ -4814,6 +4899,14 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, 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 @@ FailPathNoLock: 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. + // Only write back results on success // - _SEH2_TRY + if (NT_SUCCESS(Status)) { - *URegionSize = PRegionSize; - *UBaseAddress = (PVOID)StartingAddress; - } - _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) - { - Status = _SEH2_GetExceptionCode(); + // + // 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; } - _SEH2_END; + return Status; } -- 2.17.1