From 83a2328786294bd34836a99e0b1df206c0d0bd4c Mon Sep 17 00:00:00 2001 From: Sir Richard Date: Sun, 4 Mar 2012 06:42:49 +0000 Subject: [PATCH] [NTOS]: Blimey this was a hard one. When using the reserved flag to request 1MB in a new process (which is used for starting SMSS and CSRSS), we must request 1MB - 256 bytes (or any number, actually) to offset the fact that with a base address of 0x4, a 1MB region gets us at 0x100FFF, and not 0xFFFF, because Windows computes ending address *before* alignment, and then returns a new region size for you (so if you pass in 1MB, you get 1MB + 4096KB). In Win32csr, when the code is trying to release 1MB, this ended up in our "Case A", because it would still leave a page in the VAD. Fixed rtl to request just shy off a MB. Verified on Windows as well. [NTOS]: The fix above was due to fixing "EndingAddress" which was being initialized to zero too late (after writing to it!). This caused allocations with a fixed base address that were already on top of another allocation not to be seen as a conflict, then we tried inserting a VAD and received an ASSERT saying we've already found a VAD there. After fixing the sizing code, the bug above creeped up. Whoever wrote the NtFreeVirtualMemory test is a godsend. It has been nailing bug after bug in the VAD implementation. Thank you. svn path=/trunk/; revision=55990 --- reactos/lib/rtl/process.c | 2 +- reactos/ntoskrnl/mm/ARM3/virtual.c | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/reactos/lib/rtl/process.c b/reactos/lib/rtl/process.c index 1a23086d16d..6df7082eb13 100644 --- a/reactos/lib/rtl/process.c +++ b/reactos/lib/rtl/process.c @@ -84,7 +84,7 @@ RtlpInitEnvironment(HANDLE ProcessHandle, { /* Give 1MB starting at 0x4 */ BaseAddress = (PVOID)4; - EnviroSize = 1024 * 1024; + EnviroSize = (1024 * 1024) - 256; Status = ZwAllocateVirtualMemory(ProcessHandle, &BaseAddress, 0, diff --git a/reactos/ntoskrnl/mm/ARM3/virtual.c b/reactos/ntoskrnl/mm/ARM3/virtual.c index a013373ce78..acb08aedcfb 100644 --- a/reactos/ntoskrnl/mm/ARM3/virtual.c +++ b/reactos/ntoskrnl/mm/ARM3/virtual.c @@ -3295,6 +3295,8 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, // PRegionSize = ROUND_TO_PAGES(PRegionSize); PageCount = BYTES_TO_PAGES(PRegionSize); + EndingAddress = 0; + StartingAddress = 0; } else { @@ -3336,7 +3338,6 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, // which was passed in isn't already conflicting with an existing address // range. // - EndingAddress = 0; if (!PBaseAddress) { Status = MiFindEmptyAddressRangeInTree(PRegionSize, @@ -3345,6 +3346,17 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, (PMMADDRESS_NODE*)&Process->VadFreeHint, &StartingAddress); ASSERT(NT_SUCCESS(Status)); + + // + // Now we know where the allocation ends. Make sure it doesn't end up + // somewhere in kernel mode. + // + EndingAddress = ((ULONG_PTR)StartingAddress + PRegionSize - 1) | (PAGE_SIZE - 1); + if ((PVOID)EndingAddress > MM_HIGHEST_VAD_ADDRESS) + { + Status = STATUS_NO_MEMORY; + goto FailPath; + } } else if (MiCheckForConflictingNode(StartingAddress >> PAGE_SHIFT, EndingAddress >> PAGE_SHIFT, @@ -3357,17 +3369,6 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, goto FailPath; } - // - // Now we know where the allocation ends. Make sure it doesn't end up - // somewhere in kernel mode. - // - EndingAddress = ((ULONG_PTR)StartingAddress + PRegionSize - 1) | (PAGE_SIZE - 1); - if ((PVOID)EndingAddress > MM_HIGHEST_VAD_ADDRESS) - { - Status = STATUS_NO_MEMORY; - goto FailPath; - } - // // Write out the VAD fields for this allocation //