Ros-dev

ros-dev@reactos.org

5789 discussions

Re: [ros-dev] [ros-diffs] [tkreuzer] 64593: [NTOSKRNL] Modify MiCreatePebOrTeb to use MiInsertVadEx instead of doing everything "by hand". No, this does not "change Windows behavior". The TEB creation works exactly as befor...

by Alex Ionescu

Why do you think PEB creation cannot fail in the first place? Best regards, Alex Ionescu On Tue, Oct 7, 2014 at 5:31 PM, <tkreuzer(a)svn.reactos.org> wrote: > Author: tkreuzer > Date: Wed Oct 8 00:31:49 2014 > New Revision: 64593 > > URL: http://svn.reactos.org/svn/reactos?rev=64593&view=rev > Log: > [NTOSKRNL] > Modify MiCreatePebOrTeb to use MiInsertVadEx instead of doing everything > "by hand". No, this does not "change Windows behavior". The TEB creation > works exactly as before, and the only difference in the PEB creation is > that if the first attempt fails, we will no longer try again from the top > of the address space. But since this cannot fail in the first place, at > least not due to the VA range not being free, another attempt would be > pointless anyway! > > Modified: > trunk/reactos/ntoskrnl/mm/ARM3/procsup.c > > Modified: trunk/reactos/ntoskrnl/mm/ARM3/procsup.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/procsup.c… > > ============================================================================== > --- trunk/reactos/ntoskrnl/mm/ARM3/procsup.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/mm/ARM3/procsup.c [iso-8859-1] Wed Oct 8 > 00:31:49 2014 > @@ -50,14 +50,11 @@ > IN ULONG Size, > OUT PULONG_PTR BaseAddress) > { > - PETHREAD Thread = PsGetCurrentThread(); > PMMVAD_LONG Vad; > NTSTATUS Status; > ULONG_PTR HighestAddress, RandomBase; > ULONG AlignedSize; > LARGE_INTEGER CurrentTime; > - TABLE_SEARCH_RESULT Result = TableFoundNode; > - PMMADDRESS_NODE Parent; > > /* Allocate a VAD */ > Vad = ExAllocatePoolWithTag(NonPagedPool, sizeof(MMVAD_LONG), 'ldaV'); > @@ -70,6 +67,7 @@ > Vad->u.VadFlags.PrivateMemory = TRUE; > Vad->u.VadFlags.Protection = MM_READWRITE; > Vad->u.VadFlags.NoChange = TRUE; > + Vad->u1.Parent = NULL; > > /* Setup the secondary flags to make it a secured, writable, long VAD > */ > Vad->u2.LongFlags2 = 0; > @@ -77,10 +75,11 @@ > Vad->u2.VadFlags2.LongVad = TRUE; > Vad->u2.VadFlags2.ReadOnly = FALSE; > > - /* Lock the process address space */ > - KeAcquireGuardedMutex(&Process->AddressCreationLock); > + Vad->ControlArea = NULL; // For Memory-Area hack > + Vad->FirstPrototypePte = NULL; > > /* Check if this is a PEB creation */ > + ASSERT(sizeof(TEB) != sizeof(PEB)); > if (Size == sizeof(PEB)) > { > /* Create a random value to select one page in a 64k region */ > @@ -100,68 +99,27 @@ > > /* Calculate the highest allowed address */ > HighestAddress = RandomBase + AlignedSize - 1; > - > - /* Try to find something below the random upper margin */ > - Result = MiFindEmptyAddressRangeDownTree(ROUND_TO_PAGES(Size), > - HighestAddress, > - PAGE_SIZE, > - &Process->VadRoot, > - BaseAddress, > - &Parent); > - } > - > - /* Check for success. TableFoundNode means nothing free. */ > - if (Result == TableFoundNode) > - { > - /* For TEBs, or if a PEB location couldn't be found, scan the VAD > root */ > - Result = MiFindEmptyAddressRangeDownTree(ROUND_TO_PAGES(Size), > - > (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS, > - PAGE_SIZE, > - &Process->VadRoot, > - BaseAddress, > - &Parent); > - /* Bail out, if still nothing free was found */ > - if (Result == TableFoundNode) > - { > - KeReleaseGuardedMutex(&Process->AddressCreationLock); > - ExFreePoolWithTag(Vad, 'ldaV'); > - return STATUS_NO_MEMORY; > - } > - } > - > - /* Validate that it came from the VAD ranges */ > - ASSERT(*BaseAddress >= (ULONG_PTR)MI_LOWEST_VAD_ADDRESS); > - > - /* Build the rest of the VAD now */ > - Vad->StartingVpn = (*BaseAddress) >> PAGE_SHIFT; > - Vad->EndingVpn = ((*BaseAddress) + Size - 1) >> PAGE_SHIFT; > - Vad->u3.Secured.StartVpn = *BaseAddress; > - Vad->u3.Secured.EndVpn = (Vad->EndingVpn << PAGE_SHIFT) | (PAGE_SIZE > - 1); > - Vad->u1.Parent = NULL; > - > - /* FIXME: Should setup VAD bitmap */ > - Status = STATUS_SUCCESS; > - > - /* Pretend as if we own the working set */ > - MiLockProcessWorkingSetUnsafe(Process, Thread); > - > - /* Insert the VAD */ > - ASSERT(Vad->EndingVpn >= Vad->StartingVpn); > - Process->VadRoot.NodeHint = Vad; > - Vad->ControlArea = NULL; // For Memory-Area hack > - Vad->FirstPrototypePte = NULL; > - DPRINT("VAD: %p\n", Vad); > - DPRINT("Allocated PEB/TEB at: 0x%p for %16s\n", *BaseAddress, > Process->ImageFileName); > - MiInsertNode(&Process->VadRoot, (PVOID)Vad, Parent, Result); > - > - /* Release the working set */ > - MiUnlockProcessWorkingSetUnsafe(Process, Thread); > - > - /* Release the address space lock */ > - KeReleaseGuardedMutex(&Process->AddressCreationLock); > - > - /* Return the status */ > - return Status; > + } > + else > + { > + HighestAddress = (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS; > + } > + > + *BaseAddress = 0; > + Status = MiInsertVadEx((PMMVAD)Vad, > + BaseAddress, > + BYTES_TO_PAGES(Size), > + HighestAddress, > + PAGE_SIZE, > + MEM_TOP_DOWN); > + if (!NT_SUCCESS(Status)) > + { > + ExFreePoolWithTag(Vad, 'ldaV'); > + return STATUS_NO_MEMORY; > + } > + > + /* Success */ > + return STATUS_SUCCESS; > } > > VOID > > >

10 years, 10 months

Re: [ros-dev] [ros-diffs] [tkreuzer] 64595: [NTOSKRNL] Fix a status code

by Alex Ionescu

Would be great to have JIRA issues associated to such fixes as well as a test proving the change is correct. For example, are you merely assuming that you should return "STATUS_NO_MEMORY", or do you know for a fact that STATUS_CONFLICTING_ADDRESSES is the wrong code here? Best regards, Alex Ionescu On Wed, Oct 8, 2014 at 12:38 AM, <tkreuzer(a)svn.reactos.org> wrote: > Author: tkreuzer > Date: Wed Oct 8 07:38:56 2014 > New Revision: 64595 > > URL: http://svn.reactos.org/svn/reactos?rev=64595&view=rev > Log: > [NTOSKRNL] > Fix a status code > > Modified: > trunk/reactos/ntoskrnl/mm/ARM3/vadnode.c > > Modified: trunk/reactos/ntoskrnl/mm/ARM3/vadnode.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/vadnode.c… > > ============================================================================== > --- trunk/reactos/ntoskrnl/mm/ARM3/vadnode.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/mm/ARM3/vadnode.c [iso-8859-1] Wed Oct 8 > 07:38:56 2014 > @@ -261,7 +261,7 @@ > { > DPRINT1("Not enough free space to insert this VAD node!\n"); > KeReleaseGuardedMutex(&CurrentProcess->AddressCreationLock); > - return STATUS_CONFLICTING_ADDRESSES; > + return STATUS_NO_MEMORY; > } > > ASSERT(StartingAddress != 0); > > >

10 years, 10 months

Re: [ros-dev] [ros-diffs] [tkreuzer] 64589: [NTOSKRNL] Implement MiInsertVadEx, replacing duplicated code from NtAllocateVirtualMemory and MiMapViewOfDataSection

by Alex Ionescu

I think it would be really great that instead of combining functions together for "removing code duplication" and not even bothering to think why there's a reason why the code was duplicated, people would work on actual improvements like Jerome or Pierre, instead of re-factoring code that works just to make it 'simpler'. Especially since this introduced a few bugs which had to be later fixed. There's a million bugs in ReactOS, including in the memory manager, so fixing those seems to make more sense than refactoring. Just my 2 cents. Best regards, Alex Ionescu On Tue, Oct 7, 2014 at 5:31 PM, <tkreuzer(a)svn.reactos.org> wrote: > Author: tkreuzer > Date: Wed Oct 8 00:31:17 2014 > New Revision: 64589 > > URL: http://svn.reactos.org/svn/reactos?rev=64589&view=rev > Log: > [NTOSKRNL] > Implement MiInsertVadEx, replacing duplicated code from > NtAllocateVirtualMemory and MiMapViewOfDataSection > > Modified: > trunk/reactos/ntoskrnl/mm/ARM3/miarm.h > trunk/reactos/ntoskrnl/mm/ARM3/section.c > trunk/reactos/ntoskrnl/mm/ARM3/vadnode.c > trunk/reactos/ntoskrnl/mm/ARM3/virtual.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] Wed Oct 8 > 00:31:17 2014 > @@ -2144,6 +2144,16 @@ > IN PEPROCESS Process > ); > > +NTSTATUS > +NTAPI > +MiInsertVadEx( > + _Inout_ PMMVAD Vad, > + _In_ ULONG_PTR *BaseAddress, > + _In_ SIZE_T ViewSize, > + _In_ ULONG_PTR HighestAddress, > + _In_ ULONG_PTR Alignment, > + _In_ ULONG AllocationType); > + > VOID > NTAPI > MiInsertBasedSection( > > 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] Wed Oct 8 > 00:31:17 2014 > @@ -1252,9 +1252,7 @@ > IN ULONG AllocationType) > { > PMMVAD_LONG Vad; > - PETHREAD Thread = PsGetCurrentThread(); > - PMMSUPPORT AddressSpace; > - ULONG_PTR StartAddress, EndingAddress; > + ULONG_PTR StartAddress; > ULONG_PTR ViewSizeInPages; > PSUBSECTION Subsection; > PSEGMENT Segment; > @@ -1263,8 +1261,6 @@ > ULONG QuotaCharge = 0, QuotaExcess = 0; > PMMPTE PointerPte, LastPte; > MMPTE TempPte; > - PMMADDRESS_NODE Parent; > - TABLE_SEARCH_RESULT Result; > DPRINT("Mapping ARM3 data section\n"); > > /* Get the segment for this section */ > @@ -1361,6 +1357,7 @@ > > /* Write all the data required in the VAD for handling a fault */ > Vad->ControlArea = ControlArea; > + Vad->u.VadFlags.CommitCharge = 0; > Vad->u.VadFlags.Protection = ProtectionMask; > Vad->u2.VadFlags2.FileOffset = (ULONG)(SectionOffset->QuadPart >> 16); > Vad->u2.VadFlags2.Inherit = (InheritDisposition == ViewShare); > @@ -1437,100 +1434,23 @@ > StartAddress = 0; > } > > - /* Lock the address space and make sure the process is alive */ > - AddressSpace = MmGetCurrentAddressSpace(); > - MmLockAddressSpace(AddressSpace); > - if (Process->VmDeleted) > - { > - MmUnlockAddressSpace(AddressSpace); > - ExFreePoolWithTag(Vad, 'ldaV'); > - DPRINT1("The process is dying\n"); > - return STATUS_PROCESS_IS_TERMINATING; > - } > - > - /* Did the caller specify an address? */ > - if (StartAddress == 0) > - { > - /* ARM3 does not support these flags yet */ > - ASSERT(Process->VmTopDown == 0); > - ASSERT(ZeroBits == 0); > - > - /* Which way should we search? */ > - if (AllocationType & MEM_TOP_DOWN) > - { > - /* No, find an address top-down */ > - Result = MiFindEmptyAddressRangeDownTree(*ViewSize, > - > (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS, > - _64K, > - &Process->VadRoot, > - &StartAddress, > - &Parent); > - } > - else > - { > - /* No, find an address bottom-up */ > - Result = MiFindEmptyAddressRangeInTree(*ViewSize, > - _64K, > - &Process->VadRoot, > - &Parent, > - &StartAddress); > - } > - > - /* Check if we found a suitable location */ > - if (Result == TableFoundNode) > - { > - DPRINT1("Not enough free space to insert this section!\n"); > - MmUnlockAddressSpace(AddressSpace); > - MiDereferenceControlArea(ControlArea); > - ExFreePoolWithTag(Vad, 'ldaV'); > - return STATUS_CONFLICTING_ADDRESSES; > - } > - > - /* Get the ending address, which is the last piece we need for > the VAD */ > - EndingAddress = StartAddress + (ViewSizeInPages * PAGE_SIZE) - 1; > - } > - else > - { > - /* Get the ending address, which is the last piece we need for > the VAD */ > - EndingAddress = StartAddress + (ViewSizeInPages * PAGE_SIZE) - 1; > - > - /* Make sure it doesn't conflict with an existing allocation */ > - Result = MiCheckForConflictingNode(StartAddress >> PAGE_SHIFT, > - EndingAddress >> PAGE_SHIFT, > - &Process->VadRoot, > - &Parent); > - if (Result == TableFoundNode) > - { > - DPRINT1("Conflict with SEC_BASED or manually based > section!\n"); > - MmUnlockAddressSpace(AddressSpace); > - MiDereferenceControlArea(ControlArea); > - ExFreePoolWithTag(Vad, 'ldaV'); > - return STATUS_CONFLICTING_ADDRESSES; > - } > - } > - > - /* Now set the VAD address */ > - Vad->StartingVpn = StartAddress >> PAGE_SHIFT; > - Vad->EndingVpn = EndingAddress >> PAGE_SHIFT; > - > - /* Pretend as if we own the working set */ > - MiLockProcessWorkingSetUnsafe(Process, Thread); > - > /* Insert the VAD */ > - Process->VadRoot.NodeHint = Vad; > - MiInsertNode(&Process->VadRoot, (PVOID)Vad, Parent, Result); > - > - /* Release the working set */ > - MiUnlockProcessWorkingSetUnsafe(Process, Thread); > - > - /* Unlock the address space */ > - MmUnlockAddressSpace(AddressSpace); > + Status = MiInsertVadEx((PMMVAD)Vad, > + &StartAddress, > + ViewSizeInPages * PAGE_SIZE, > + MAXULONG_PTR >> ZeroBits, > + MM_VIRTMEM_GRANULARITY, > + AllocationType); > + if (!NT_SUCCESS(Status)) > + { > + return Status; > + } > > /* Windows stores this for accounting purposes, do so as well */ > if (!Segment->u2.FirstMappedVa) Segment->u2.FirstMappedVa = > (PVOID)StartAddress; > > /* Finally, let the caller know where, and for what size, the view > was mapped */ > - *ViewSize = (ULONG_PTR)EndingAddress - (ULONG_PTR)StartAddress + 1; > + *ViewSize = ViewSizeInPages * PAGE_SIZE; > *BaseAddress = (PVOID)StartAddress; > DPRINT("Start and region: 0x%p, 0x%p\n", *BaseAddress, *ViewSize); > return STATUS_SUCCESS; > > Modified: trunk/reactos/ntoskrnl/mm/ARM3/vadnode.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/vadnode.c… > > ============================================================================== > --- trunk/reactos/ntoskrnl/mm/ARM3/vadnode.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/mm/ARM3/vadnode.c [iso-8859-1] Wed Oct 8 > 00:31:17 2014 > @@ -193,6 +193,138 @@ > MiInsertNode(&Process->VadRoot, (PVOID)Vad, Parent, Result); > } > > +NTSTATUS > +NTAPI > +MiInsertVadEx( > + _Inout_ PMMVAD Vad, > + _In_ ULONG_PTR *BaseAddress, > + _In_ SIZE_T ViewSize, > + _In_ ULONG_PTR HighestAddress, > + _In_ ULONG_PTR Alignment, > + _In_ ULONG AllocationType) > +{ > + ULONG_PTR StartingAddress, EndingAddress; > + PEPROCESS CurrentProcess; > + PETHREAD CurrentThread; > + TABLE_SEARCH_RESULT Result; > + PMMADDRESS_NODE Parent; > + > + /* Align the view size to pages */ > + ViewSize = ALIGN_UP_BY(ViewSize, PAGE_SIZE); > + > + /* Get the current process */ > + CurrentProcess = PsGetCurrentProcess(); > + > + /* Acquire the address creation lock and make sure the process is > alive */ > + KeAcquireGuardedMutex(&CurrentProcess->AddressCreationLock); > + if (CurrentProcess->VmDeleted) > + { > + KeReleaseGuardedMutex(&CurrentProcess->AddressCreationLock); > + DPRINT1("The process is dying\n"); > + return STATUS_PROCESS_IS_TERMINATING; > + } > + > + /* Did the caller specify an address? */ > + if (*BaseAddress == 0) > + { > + /* Make sure HighestAddress is not too large */ > + HighestAddress = min(HighestAddress, > (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS); > + > + /* Which way should we search? */ > + if ((AllocationType & MEM_TOP_DOWN) || CurrentProcess->VmTopDown) > + { > + /* Find an address top-down */ > + Result = MiFindEmptyAddressRangeDownTree(ViewSize, > + HighestAddress, > + Alignment, > + > &CurrentProcess->VadRoot, > + &StartingAddress, > + &Parent); > + } > + else > + { > + /* Find an address bottom-up */ > + Result = MiFindEmptyAddressRangeInTree(ViewSize, > + Alignment, > + > &CurrentProcess->VadRoot, > + &Parent, > + &StartingAddress); > + } > + > + /* Get the ending address, which is the last piece we need for > the VAD */ > + EndingAddress = StartingAddress + ViewSize - 1; > + > + /* Check if we found a suitable location */ > + if ((Result == TableFoundNode) || (EndingAddress > > HighestAddress)) > + { > + DPRINT1("Not enough free space to insert this VAD node!\n"); > + KeReleaseGuardedMutex(&CurrentProcess->AddressCreationLock); > + return STATUS_CONFLICTING_ADDRESSES; > + } > + > + ASSERT(StartingAddress != 0); > + ASSERT(StartingAddress < (ULONG_PTR)HighestAddress); > + ASSERT(EndingAddress > StartingAddress); > + } > + else > + { > + /* Calculate the starting and ending address */ > + StartingAddress = ALIGN_DOWN_BY(*BaseAddress, Alignment); > + EndingAddress = StartingAddress + ViewSize - 1; > + > + /* Make sure it doesn't conflict with an existing allocation */ > + Result = MiCheckForConflictingNode(StartingAddress >> PAGE_SHIFT, > + EndingAddress >> PAGE_SHIFT, > + &CurrentProcess->VadRoot, > + &Parent); > + if (Result == TableFoundNode) > + { > + DPRINT1("Given address conflicts with existing node\n"); > + KeReleaseGuardedMutex(&CurrentProcess->AddressCreationLock); > + return STATUS_CONFLICTING_ADDRESSES; > + } > + } > + > + /* Now set the VAD address */ > + Vad->StartingVpn = StartingAddress >> PAGE_SHIFT; > + Vad->EndingVpn = EndingAddress >> PAGE_SHIFT; > + > + /* Check if the VAD is to be secured */ > + if (Vad->u2.VadFlags2.OneSecured) > + { > + /* This *must* be a long VAD! */ > + ASSERT(Vad->u2.VadFlags2.LongVad); > + > + /* Yeah this is retarded, I didn't invent it! */ > + ((PMMVAD_LONG)Vad)->u3.Secured.StartVpn = StartingAddress; > + ((PMMVAD_LONG)Vad)->u3.Secured.EndVpn = EndingAddress; > + } > + > + /* Lock the working set */ > + CurrentThread = PsGetCurrentThread(); > + MiLockProcessWorkingSetUnsafe(CurrentProcess, CurrentThread); > + > + /* Insert the VAD */ > + CurrentProcess->VadRoot.NodeHint = Vad; > + MiInsertNode(&CurrentProcess->VadRoot, (PVOID)Vad, Parent, Result); > + > + /* Release the working set */ > + MiUnlockProcessWorkingSetUnsafe(CurrentProcess, CurrentThread); > + > + /* Update the process' virtual size, and peak virtual size */ > + CurrentProcess->VirtualSize += ViewSize; > + if (CurrentProcess->VirtualSize > CurrentProcess->PeakVirtualSize) > + { > + CurrentProcess->PeakVirtualSize = CurrentProcess->VirtualSize; > + } > + > + /* Unlock the address space */ > + KeReleaseGuardedMutex(&CurrentProcess->AddressCreationLock); > + > + *BaseAddress = StartingAddress; > + return STATUS_SUCCESS; > +} > + > VOID > NTAPI > MiInsertBasedSection(IN PSECTION Section) > > 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] Wed Oct 8 > 00:31:17 2014 > @@ -4304,12 +4304,12 @@ > { > PEPROCESS Process; > PMEMORY_AREA MemoryArea; > - PFN_NUMBER PageCount; > PMMVAD Vad = NULL, FoundVad; > NTSTATUS Status; > PMMSUPPORT AddressSpace; > PVOID PBaseAddress; > - ULONG_PTR PRegionSize, StartingAddress, EndingAddress, HighestAddress; > + ULONG_PTR PRegionSize, StartingAddress, EndingAddress; > + ULONG_PTR HighestAddress = (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS; > PEPROCESS CurrentProcess = PsGetCurrentProcess(); > KPROCESSOR_MODE PreviousMode = KeGetPreviousMode(); > PETHREAD CurrentThread = PsGetCurrentThread(); > @@ -4319,7 +4319,6 @@ > MMPTE TempPte; > PMMPTE PointerPte, PointerPde, LastPte; > TABLE_SEARCH_RESULT Result; > - PMMADDRESS_NODE Parent; > PAGED_CODE(); > > /* Check for valid Zero bits */ > @@ -4544,7 +4543,6 @@ > // This is a blind commit, all we need is the region size > // > PRegionSize = ROUND_TO_PAGES(PRegionSize); > - PageCount = BYTES_TO_PAGES(PRegionSize); > EndingAddress = 0; > StartingAddress = 0; > > @@ -4563,13 +4561,6 @@ > goto FailPathNoLock; > } > } > - else > - { > - // > - // Use the highest VAD address as maximum > - // > - HighestAddress = (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS; > - } > } > else > { > @@ -4579,8 +4570,8 @@ > // 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); > - PageCount = BYTES_TO_PAGES(EndingAddress - StartingAddress); > + PRegionSize = EndingAddress + 1 - > ROUND_DOWN((ULONG_PTR)PBaseAddress, _64K); > + StartingAddress = (ULONG_PTR)PBaseAddress; > } > > // > @@ -4594,7 +4585,7 @@ > goto FailPathNoLock; > } > > - Vad->u.LongFlags = 0; > + RtlZeroMemory(Vad, sizeof(MMVAD_LONG)); > if (AllocationType & MEM_COMMIT) Vad->u.VadFlags.MemCommit = 1; > Vad->u.VadFlags.Protection = ProtectionMask; > Vad->u.VadFlags.PrivateMemory = 1; > @@ -4602,124 +4593,24 @@ > Vad->ControlArea = NULL; // For Memory-Area hack > > // > - // Lock the address space and make sure the process isn't already > dead > - // > - AddressSpace = MmGetCurrentAddressSpace(); > - MmLockAddressSpace(AddressSpace); > - if (Process->VmDeleted) > - { > - Status = STATUS_PROCESS_IS_TERMINATING; > - goto FailPath; > - } > - > - // > - // Did we have a base address? If no, find a valid address that > is 64KB > - // aligned in the VAD tree. Otherwise, make sure that the address > range > - // which was passed in isn't already conflicting with an existing > address > - // range. > - // > - if (!PBaseAddress) > - { > - /* Which way should we search? */ > - if ((AllocationType & MEM_TOP_DOWN) || Process->VmTopDown) > - { > - /* Find an address top-down */ > - Result = MiFindEmptyAddressRangeDownTree(PRegionSize, > - HighestAddress, > - _64K, > - > &Process->VadRoot, > - &StartingAddress, > - &Parent); > - } > - else > - { > - /* Find an address bottom-up */ > - Result = MiFindEmptyAddressRangeInTree(PRegionSize, > - _64K, > - &Process->VadRoot, > - &Parent, > - &StartingAddress); > - } > - > - if (Result == TableFoundNode) > - { > - Status = STATUS_NO_MEMORY; > - goto FailPath; > - } > - > - // > - // Now we know where the allocation ends. Make sure it > doesn't end up > - // somewhere in kernel mode. > - // > - ASSERT(StartingAddress != 0); > - ASSERT(StartingAddress < (ULONG_PTR)MM_HIGHEST_USER_ADDRESS); > - EndingAddress = (StartingAddress + PRegionSize - 1) | > (PAGE_SIZE - 1); > - ASSERT(EndingAddress > StartingAddress); > - if (EndingAddress > HighestAddress) > - { > - Status = STATUS_NO_MEMORY; > - goto FailPath; > - } > - } > - else > - { > - /* Make sure it doesn't conflict with an existing allocation > */ > - Result = MiCheckForConflictingNode(StartingAddress >> > PAGE_SHIFT, > - EndingAddress >> > PAGE_SHIFT, > - &Process->VadRoot, > - &Parent); > - if (Result == TableFoundNode) > - { > - // > - // The address specified is in conflict! > - // > - Status = STATUS_CONFLICTING_ADDRESSES; > - goto FailPath; > - } > - } > - > - // > - // Write out the VAD fields for this allocation > - // > - Vad->StartingVpn = StartingAddress >> PAGE_SHIFT; > - Vad->EndingVpn = EndingAddress >> PAGE_SHIFT; > - > - // > - // FIXME: Should setup VAD bitmap > - // > - Status = STATUS_SUCCESS; > - > - // > - // Lock the working set and insert the VAD into the process VAD > tree > - // > - MiLockProcessWorkingSetUnsafe(Process, CurrentThread); > - Process->VadRoot.NodeHint = Vad; > - 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 > - // this. > - // > - Process->VirtualSize += PRegionSize; > - if (Process->VirtualSize > Process->PeakVirtualSize) > - { > - Process->PeakVirtualSize = Process->VirtualSize; > - } > - > - // > - // Release address space and detach and dereference the target > process if > + // Insert the VAD > + // > + Status = MiInsertVadEx(Vad, > + &StartingAddress, > + PRegionSize, > + HighestAddress, > + MM_VIRTMEM_GRANULARITY, > + AllocationType); > + if (!NT_SUCCESS(Status)) > + { > + DPRINT1("Failed to insert the VAD!\n"); > + goto FailPathNoLock; > + } > + > + // > + // Detach and dereference the target process if > // it was different from the current process > // > - MmUnlockAddressSpace(AddressSpace); > if (Attached) KeUnstackDetachProcess(&ApcState); > if (ProcessHandle != NtCurrentProcess()) > ObDereferenceObject(Process); > > > >

10 years, 10 months

Re: [ros-dev] [ros-diffs] [aandrejevic] 64656: [FAST486] Update the copyright year. Implement the first batch of FPU instructions. These are still experimental.

by Timo Kreuzer

Seriously? Am 11.10.2014 01:07, schrieb aandrejevic(a)svn.reactos.org: > + /* Count the leading zeros */ > + while (!(Data->Mantissa & (1 << (63 - LeadingZeros)))) LeadingZeros++;

10 years, 10 months

Re: [ros-dev] EPIC: ReactOS@Samsung Galaxy S III mini

by Love Nystrom

@Timo Yes of course.. It was a fleeting idea it would be cute to run on a computer 5x10x1 cm. ;-) I didn't realize the ARM branch was in poor condition though. But it stands to reason.. Porting Intel assembly to ARM assembly and considering all the archiectural differences is/would be a gargantuan job. Keep up the good work.. Love On 2014-10-09 19.00, ros-dev-request(a)reactos.org wrote: > Guys, I don't want to spoil your hopes, but native ARM support is far > away from being usable. It doesn't even compile. There is no native ARM! > And there won't be for ages, until we have a really knowledged ARM > hacker, with the kernel and hardware knowledge of Alex, who would invest > a huge lot of time into getting it up and running. So unless we want to > hire someone fulltime for at least half a year, who would probably like > to be paid a normal software engineer wage, which we cannot afford, we > won't have ARM support anytime soon. > x64 is a much more reasonable task, mostly lacking stuff in Mm. > And that will still require a lot of work. > And I have more than enough other things to do

10 years, 10 months

Re: [ros-dev] [ros-diffs] [pschweitzer] 64617: [NTFS] - Add a directory entry into FCB which will be a copy of the $FILE_NAME attribute of the FILE record. Though, it won't contain the name itself (in the FCB instead), so k...

by Thomas Faber

On 2014-10-08 21:32, pschweitzer(a)svn.reactos.org wrote: > + MftRecord = ExAllocatePoolWithTag(NonPagedPool, > + Vcb->NtfsInfo.BytesPerFileRecord, > + TAG_NTFS); > + if (MftRecord == NULL) > + { > + return NULL; > + } > + > + if (!NT_SUCCESS(ReadFileRecord(Vcb, NTFS_FILE_ROOT, MftRecord))) > + { > + return NULL; > + } > + > + FileName = GetFileNameFromRecord(MftRecord); > + if (!FileName) > + { > + return NULL; > + } > > Fcb = NtfsCreateFCB(L"\\", Vcb); > - > -// memset(Fcb->entry.Filename, ' ', 11); > - > -// Fcb->Entry.DataLengthL = Vcb->CdInfo.RootSize; > -// Fcb->Entry.ExtentLocationL = Vcb->CdInfo.RootStart; > -// Fcb->Entry.FileFlags = 0x02; // FILE_ATTRIBUTE_DIRECTORY; > + if (!Fcb) > + { > + return NULL; > + } These failure cases look like they're all at least leaking MftRecord :p

10 years, 10 months

Re: [ros-dev] [ros-diffs] [jgardou] 64525: [KERNEL32] - Always allocate a guard page at the bottom of the stack. It doesn't depend on it being reserved or committed, it just has to be here.

by Alex Ionescu

Hi, This code changes Windows behavior. Best regards, Alex Ionescu On Sat, Oct 4, 2014 at 1:25 PM, <jgardou(a)svn.reactos.org> wrote: > Author: jgardou > Date: Sat Oct 4 20:25:35 2014 > New Revision: 64525 > > URL: http://svn.reactos.org/svn/reactos?rev=64525&view=rev > Log: > [KERNEL32] > - Always allocate a guard page at the bottom of the stack. > It doesn't depend on it being reserved or committed, it just has to be > here. > > Modified: > trunk/reactos/dll/win32/kernel32/client/utils.c > > Modified: trunk/reactos/dll/win32/kernel32/client/utils.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/… > > ============================================================================== > --- trunk/reactos/dll/win32/kernel32/client/utils.c [iso-8859-1] > (original) > +++ trunk/reactos/dll/win32/kernel32/client/utils.c [iso-8859-1] Sat > Oct 4 20:25:35 2014 > @@ -358,7 +358,6 @@ > NTSTATUS Status; > PIMAGE_NT_HEADERS Headers; > ULONG_PTR Stack; > - BOOLEAN UseGuard; > ULONG PageSize, Dummy, AllocationGranularity; > SIZE_T StackReserveHeader, StackCommitHeader, GuardPageSize, > GuaranteedStackCommit; > DPRINT("BaseCreateStack (hProcess: %p, Max: %lx, Current: %lx)\n", > @@ -426,18 +425,6 @@ > /* Update the Stack Position */ > Stack += StackReserve - StackCommit; > > - /* Check if we will need a guard page */ > - if (StackReserve > StackCommit) > - { > - Stack -= PageSize; > - StackCommit += PageSize; > - UseGuard = TRUE; > - } > - else > - { > - UseGuard = FALSE; > - } > - > /* Allocate memory for the stack */ > Status = NtAllocateVirtualMemory(hProcess, > (PVOID*)&Stack, > @@ -457,25 +444,21 @@ > InitialTeb->StackLimit = (PVOID)Stack; > > /* Create a guard page */ > - if (UseGuard) > - { > - /* Set the guard page */ > - GuardPageSize = PAGE_SIZE; > - Status = NtProtectVirtualMemory(hProcess, > - (PVOID*)&Stack, > - &GuardPageSize, > - PAGE_GUARD | PAGE_READWRITE, > - &Dummy); > - if (!NT_SUCCESS(Status)) > - { > - DPRINT1("Failure to set guard page\n"); > - return Status; > - } > - > - /* Update the Stack Limit keeping in mind the Guard Page */ > - InitialTeb->StackLimit = > (PVOID)((ULONG_PTR)InitialTeb->StackLimit + > - GuardPageSize); > - } > + GuardPageSize = PageSize; > + Status = NtProtectVirtualMemory(hProcess, > + (PVOID*)&Stack, > + &GuardPageSize, > + PAGE_GUARD | PAGE_READWRITE, > + &Dummy); > + if (!NT_SUCCESS(Status)) > + { > + DPRINT1("Failure to set guard page\n"); > + return Status; > + } > + > + /* Update the Stack Limit keeping in mind the Guard Page */ > + InitialTeb->StackLimit = (PVOID)((ULONG_PTR)InitialTeb->StackLimit + > + GuardPageSize); > > /* We are done! */ > return STATUS_SUCCESS; > > >

10 years, 10 months

Re: [ros-dev] [ros-diffs] [tkreuzer] 64449: [DLLEXPORT_TEST] Fix a typo

by Thomas Faber

On 2014-10-02 09:30, tkreuzer(a)svn.reactos.org wrote: > -add_executable(dllimport_test > +add_library(dllimport_test > dllimport_framedyn.cpp) Isn't the point of this test to check for linker errors?

10 years, 10 months

Re: [ros-dev] [ros-diffs] [jgardou] 64396: [NTOS/CM] - Add NtQueryKey(KeyNameInformation) implementation. CORE-8581 #resolve

by Timo Kreuzer

A few remarks: - There should be only one SEH, not for performance, but since the KeyNameInfo->NameLength = 0; needs to be in SEH as well - The function duplicates a lot of code from CmpConstructName, possibly even incomplete - There was already an implementation in the kernel-fun branch Unrelated to your code, just something I spotted in that file: - CmpQueryKeyData cannot be wrapped in SEH (it would cause reference leaks on exception), but is wrapped in SEH in CmEnumerateKey - CmpQueryKeyData does not use SEH, but is called with user mode pointers from CmQueryKey Timo Am 29.09.2014 18:27, schrieb jgardou(a)svn.reactos.org: > Author: jgardou > Date: Mon Sep 29 16:27:16 2014 > New Revision: 64396 > > URL: http://svn.reactos.org/svn/reactos?rev=64396&view=rev > Log: > [NTOS/CM] > - Add NtQueryKey(KeyNameInformation) implementation. > CORE-8581 #resolve > >

10 years, 10 months

Re: [ros-dev] [ros-diffs] [jgardou] 64392: [NDK] - Fix KEY_NAME_INFORMATION structure

by Timo Kreuzer

Wow, ... IMO we should autogenerate a header for usermode stuff from XDK, instead of using NDK. Am 29.09.2014 15:32, schrieb jgardou(a)svn.reactos.org: > Author: jgardou > Date: Mon Sep 29 13:32:44 2014 > New Revision: 64392 > > URL: http://svn.reactos.org/svn/reactos?rev=64392&view=rev > Log: > [NDK] > - Fix KEY_NAME_INFORMATION structure > > Modified: > trunk/reactos/include/ndk/cmtypes.h > > Modified: trunk/reactos/include/ndk/cmtypes.h > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/ndk/cmtypes.h?rev=… > ============================================================================== > --- trunk/reactos/include/ndk/cmtypes.h [iso-8859-1] (original) > +++ trunk/reactos/include/ndk/cmtypes.h [iso-8859-1] Mon Sep 29 13:32:44 2014 > @@ -296,6 +296,7 @@ > > typedef struct _KEY_NAME_INFORMATION > { > + ULONG NameLength; > WCHAR Name[1]; > } KEY_NAME_INFORMATION, *PKEY_NAME_INFORMATION; > > > >

10 years, 10 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev