Ros-dev October 2014

ros-dev@reactos.org

15 participants
38 discussions

Re: [ros-dev] [ros-diffs] [tfaber] 64749: [PSDK] - Use macro version of RtlUlonglongByteSwap in winternl.h because using the fastcall version causes stack corruption CORE-8632 #resolve

by Ged Murphy

There shouldn't really be anything using this header -----Original Message----- From: Ros-diffs [mailto:ros-diffs-bounces@reactos.org] On Behalf Of tfaber(a)svn.reactos.org Sent: 15 October 2014 17:38 To: ros-diffs(a)reactos.org Subject: [ros-diffs] [tfaber] 64749: [PSDK] - Use macro version of RtlUlonglongByteSwap in winternl.h because using the fastcall version causes stack corruption CORE-8632 #resolve Author: tfaber Date: Wed Oct 15 16:38:13 2014 New Revision: 64749 URL: http://svn.reactos.org/svn/reactos?rev=64749&view=rev Log: [PSDK] - Use macro version of RtlUlonglongByteSwap in winternl.h because using the fastcall version causes stack corruption CORE-8632 #resolve Modified: trunk/reactos/include/psdk/winternl.h Modified: trunk/reactos/include/psdk/winternl.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/psdk/winternl.h?re… ============================================================================== --- trunk/reactos/include/psdk/winternl.h [iso-8859-1] (original) +++ trunk/reactos/include/psdk/winternl.h [iso-8859-1] Wed Oct 15 16:38:13 2014 @@ -2310,7 +2310,12 @@ BOOLEAN WINAPI RtlTimeToSecondsSince1980(const LARGE_INTEGER *,LPDWORD); BOOL WINAPI RtlTryEnterCriticalSection(RTL_CRITICAL_SECTION *); +#ifdef __REACTOS__ ULONGLONG __fastcall RtlUlonglongByteSwap(ULONGLONG); +#define RtlUlonglongByteSwap(_x) _byteswap_uint64((_x)) #else ULONGLONG +__cdecl RtlUlonglongByteSwap(ULONGLONG); #endif DWORD WINAPI RtlUnicodeStringToAnsiSize(const UNICODE_STRING*); NTSTATUS WINAPI RtlUnicodeStringToAnsiString(PANSI_STRING,PCUNICODE_STRING,BOOLEAN); NTSTATUS WINAPI RtlUnicodeStringToInteger(const UNICODE_STRING *,ULONG,ULONG *);

10 years, 2 months

Re: [ros-dev] [ros-diffs] [tfaber] 64762: [NPFS] - Don't call RtlEqualUnicodeString (paged code) while holding a spin lock. Powered by Driver Verifier.

by Hermès BÉLUSCA - MAÏTO

What is the *root* reason for this problem (ok Ive seen in the comment, paged code and holding spin lock, but it doesnt answer my question)? Because Ive looked at our code of RtlEqualUnicodeString and I see that it just calls RtlCompareUnicodeString that in turns just loops over the two memory buffers, checks their contents (and in case we check for char case, calls RtlUpcaseUnicodeChar). And RtlCompareMemory does that, too? What Im missing? Hermès. ------------------------------------------------------------------------- Author: tfaber Date: Thu Oct 16 16:40:13 2014 New Revision: 64762 URL: http://svn.reactos.org/svn/reactos?rev=64762&view=rev Log: [NPFS] - Don't call RtlEqualUnicodeString (paged code) while holding a spin lock. Powered by Driver Verifier. Modified: trunk/reactos/drivers/filesystems/npfs/waitsup.c Modified: trunk/reactos/drivers/filesystems/npfs/waitsup.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/npfs/wa itsup.c?rev=64762&r1=64761&r2=64762&view=diff ============================================================================ == --- trunk/reactos/drivers/filesystems/npfs/waitsup.c [iso-8859-1] (original) +++ trunk/reactos/drivers/filesystems/npfs/waitsup.c [iso-8859-1] Thu Oct 16 16:40:13 2014 @@ -97,6 +97,22 @@ { InitializeListHead(&WaitQueue->WaitList); KeInitializeSpinLock(&WaitQueue->WaitLock); +} + +static +BOOLEAN +NpEqualUnicodeString(IN PCUNICODE_STRING String1, + IN PCUNICODE_STRING String2) +{ + SIZE_T EqualLength; + + if (String1->Length != String2->Length) + return FALSE; + + EqualLength = RtlCompareMemory(String1->Buffer, + String2->Buffer, + String1->Length); + return EqualLength == String1->Length; } NTSTATUS @@ -156,7 +172,8 @@ PipeName.MaximumLength = PipeName.Length; } - if (RtlEqualUnicodeString(&WaitName, &PipeName, FALSE)) + /* Can't use RtlEqualUnicodeString with a spinlock held */ + if (NpEqualUnicodeString(&WaitName, &PipeName)) { /* Found a matching wait. Cancel it */ RemoveEntryList(&WaitIrp->Tail.Overlay.ListEntry);

10 years, 2 months

Re: [ros-dev] [ros-diffs] [tkreuzer] 64757: [NTOSKRNL] Don't use an uninitialized variable in MmArmAccessFault (Alex, please review). Brought to you by MSVC runtime checks.

by Alex Ionescu

Wow, does static analyzer really not catch this? Best regards, Alex Ionescu On Wed, Oct 15, 2014 at 3:03 PM, <tkreuzer(a)svn.reactos.org> wrote: > Author: tkreuzer > Date: Wed Oct 15 22:03:50 2014 > New Revision: 64757 > > URL: http://svn.reactos.org/svn/reactos?rev=64757&view=rev > Log: > [NTOSKRNL] > Don't use an uninitialized variable in MmArmAccessFault (Alex, please > review). Brought to you by MSVC runtime checks. > > Modified: > trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c > > Modified: trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pagfault.… > > ============================================================================== > --- trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] Wed Oct 15 > 22:03:50 2014 > @@ -1511,7 +1511,7 @@ > NTSTATUS Status; > PMMSUPPORT WorkingSet; > ULONG ProtectionCode; > - PMMVAD Vad; > + PMMVAD Vad = NULL; > PFN_NUMBER PageFrameIndex; > ULONG Color; > BOOLEAN IsSessionAddress; > > >

10 years, 2 months

Re: [ros-dev] [ros-diffs] [pschweitzer] 64752: [NTFS] Implement NtfsDateTimeToFileTime() which convert epoch time (1970) to Windows time (1601)

by Thomas Faber

On 2014-10-15 22:23, pschweitzer(a)svn.reactos.org wrote: > +/* See: > + -> http://msdn.microsoft.com/en-us/library/ms724228 > + -> http://bos.asmhackers.net/docs/filesystems/ntfs/standard.html#layout > + */ > +VOID > +NtfsDateTimeToFileTime(ULONGLONG NtfsTime, > + PLARGE_INTEGER SystemTime) > +{ > + > + SystemTime->QuadPart = NtfsTime + 116444736000000000; > +} Doesn't NTFS use FILETIME directly? I thought that's the reason it's called "file time" in the first place. ;) Wikipedia says "Date range: 1 January 1601 – 28 May 60056 (File times are 64-bit numbers counting 100-nanosecond intervals (ten million per second) since 1601, which is 58,000+ years)" and your link doesn't seem to disagree.

10 years, 2 months

Re: [ros-dev] [ros-diffs] [tfaber] 64695: [ROSAUTOTEST] - DbgPrint expects a format string! ROSTESTS-145 #resolve

by Colin Finck

tfaber(a)svn.reactos.org wrote: > - DbgPrint(DbgString); > + OutputDebugStringA(DbgString); FYI, we had OutputDebugStringA there in the first place, but I changed it to DbgPrint in r40147. IIRC, output from DbgPrint was directly sent to the debug port while OutputDebugStringA buffers the data first. This is especially bad, when a test crashes the OS and we lose several kilobytes of log data before they are printed out. Probably, a DbgPrint("%s", DbgString) is the better alternative here. - Colin

10 years, 2 months

Re: [ros-dev] [ros-diffs] [tkreuzer] 64658: [NTDLL] Don't assert that the caller of exported APIs passes correct parameters.

by Alex Ionescu

Windows does. Why shouldn't we? It's a non-documented API. Best regards, Alex Ionescu On Sat, Oct 11, 2014 at 1:52 AM, <tkreuzer(a)svn.reactos.org> wrote: > Author: tkreuzer > Date: Sat Oct 11 08:52:33 2014 > New Revision: 64658 > > URL: http://svn.reactos.org/svn/reactos?rev=64658&view=rev > Log: > [NTDLL] > Don't assert that the caller of exported APIs passes correct parameters. > > Modified: > trunk/reactos/dll/ntdll/ldr/ldrapi.c > > Modified: trunk/reactos/dll/ntdll/ldr/ldrapi.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/dll/ntdll/ldr/ldrapi.c?rev… > > ============================================================================== > --- trunk/reactos/dll/ntdll/ldr/ldrapi.c [iso-8859-1] (original) > +++ trunk/reactos/dll/ntdll/ldr/ldrapi.c [iso-8859-1] Sat Oct 11 > 08:52:33 2014 > @@ -209,9 +209,6 @@ > /* A normal failure */ > return STATUS_INVALID_PARAMETER_3; > } > - > - /* Do or Do Not. There is no Try */ > - ASSERT((Disposition != NULL) || !(Flags & > LDR_LOCK_LOADER_LOCK_FLAG_TRY_ONLY)); > > /* If the flag is set, make sure we have a valid pointer to use */ > if ((Flags & LDR_LOCK_LOADER_LOCK_FLAG_TRY_ONLY) && !(Disposition)) > > >

10 years, 2 months

Re: [ros-dev] [ros-diffs] [tfaber] 64665: [NTOS:KE] - Implement KiRaiseSecurityCheckFailure[Handler] to handle int 0x29 (__fastfail). Based on patch by Timo Kreuzer. (Yes, this is a Windows 8 feature. However all it does is...

by Alex Ionescu

Now improve the LIST_ENTRY Macros to use it :) Best regards, Alex Ionescu On Sat, Oct 11, 2014 at 6:15 AM, <tfaber(a)svn.reactos.org> wrote: > Author: tfaber > Date: Sat Oct 11 13:15:10 2014 > New Revision: 64665 > > URL: http://svn.reactos.org/svn/reactos?rev=64665&view=rev > Log: > [NTOS:KE] > - Implement KiRaiseSecurityCheckFailure[Handler] to handle int 0x29 > (__fastfail). Based on patch by Timo Kreuzer. > (Yes, this is a Windows 8 feature. However all it does is improve the > debugging experience, and we have a need for that) > CORE-8419 > > Modified: > trunk/reactos/include/reactos/mc/bugcodes.mc > trunk/reactos/ntoskrnl/ke/i386/trap.s > trunk/reactos/ntoskrnl/ke/i386/traphdlr.c > > Modified: trunk/reactos/include/reactos/mc/bugcodes.mc > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/include/reactos/mc/bugcode… > > ============================================================================== > --- trunk/reactos/include/reactos/mc/bugcodes.mc [iso-8859-1] > (original) > +++ trunk/reactos/include/reactos/mc/bugcodes.mc [iso-8859-1] Sat > Oct 11 13:15:10 2014 > @@ -1128,7 +1128,7 @@ > Run a system diagnostic utility supplied by your hardware manufacturer. > In particular, run a memory check, and check for faulty or mismatched > memory. Try changing video adapters. > - > + > Disable or remove any newly installed hardware and drivers. Disable or > remove any newly installed software. If you need to use Safe Mode to > remove or disable components, restart your computer, press F8 to select > @@ -1322,7 +1322,7 @@ > SymbolicName=DRIVER_CORRUPTED_EXPOOL > Language=English > A device driver has pool. > - > + > Check to make sure any new hardware or software is properly installed. > If this is a new installation, ask your hardware or software manufacturer > for any ReactOS updates you might need. > @@ -1478,7 +1478,7 @@ > must not contain such items. Usually this is memory being freed. This > is usually caused by a device driver that has not cleaned up properly > before freeing memory. > - > + > If Parameter1 == 1, an attempt was made to queue an executive worker item > with a usermode execution routine. > . > @@ -1570,3 +1570,11 @@ > Language=English > An attempt was made to execute to non-executable memory. > . > + > +MessageId=0x139 > +Severity=Success > +Facility=System > +SymbolicName=KERNEL_SECURITY_CHECK_FAILURE > +Language=English > +A critical kernel security check failed. > +. > > Modified: trunk/reactos/ntoskrnl/ke/i386/trap.s > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/trap.s?re… > > ============================================================================== > --- trunk/reactos/ntoskrnl/ke/i386/trap.s [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/ke/i386/trap.s [iso-8859-1] Sat Oct 11 > 13:15:10 2014 > @@ -59,9 +59,11 @@ > idt _KiTrap11, INT_32_DPL0 /* INT 11: Align Check Exception > (#AC) */ > idt _KiTrap0F, INT_32_DPL0 /* INT 12: Machine Check Exception > (#MC)*/ > idt _KiTrap0F, INT_32_DPL0 /* INT 13: SIMD FPU Exception (#XF) > */ > -REPEAT 22 > -idt _KiTrap0F, INT_32_DPL0 /* INT 14-29: UNDEFINED INTERRUPTS > */ > +REPEAT 21 > +idt _KiTrap0F, INT_32_DPL0 /* INT 14-28: UNDEFINED INTERRUPTS > */ > ENDR > +idt _KiRaiseSecurityCheckFailure, INT_32_DPL3 > + /* INT 29: Handler for __fastfail > */ > idt _KiGetTickCount, INT_32_DPL3 /* INT 2A: Get Tick Count Handler > */ > idt _KiCallbackReturn, INT_32_DPL3 /* INT 2B: User-Mode Callback Return > */ > idt _KiRaiseAssertion, INT_32_DPL3 /* INT 2C: Debug Assertion Handler > */ > @@ -113,6 +115,7 @@ > TRAP_ENTRY KiTrap10, KI_PUSH_FAKE_ERROR_CODE > TRAP_ENTRY KiTrap11, KI_PUSH_FAKE_ERROR_CODE > TRAP_ENTRY KiTrap13, KI_PUSH_FAKE_ERROR_CODE > +TRAP_ENTRY KiRaiseSecurityCheckFailure, KI_PUSH_FAKE_ERROR_CODE > TRAP_ENTRY KiGetTickCount, KI_PUSH_FAKE_ERROR_CODE > TRAP_ENTRY KiCallbackReturn, KI_PUSH_FAKE_ERROR_CODE > TRAP_ENTRY KiRaiseAssertion, KI_PUSH_FAKE_ERROR_CODE > > Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/traphdlr.… > > ============================================================================== > --- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] Sat Oct 11 > 13:15:10 2014 > @@ -1462,6 +1462,46 @@ > > VOID > FASTCALL > +KiRaiseSecurityCheckFailureHandler(IN PKTRAP_FRAME TrapFrame) > +{ > + /* Save trap frame */ > + KiEnterTrap(TrapFrame); > + > + /* Decrement EIP to point to the INT29 instruction (2 bytes, not 1 > like INT3) */ > + TrapFrame->Eip -= 2; > + > + /* Check if this is a user trap */ > + if (KiUserTrap(TrapFrame)) > + { > + /* Dispatch exception to user mode */ > + KiDispatchException1Args(STATUS_STACK_BUFFER_OVERRUN, > + TrapFrame->Eip, > + TrapFrame->Ecx, > + TrapFrame); > + } > + else > + { > + EXCEPTION_RECORD ExceptionRecord; > + > + /* Bugcheck the system */ > + ExceptionRecord.ExceptionCode = STATUS_STACK_BUFFER_OVERRUN; > + ExceptionRecord.ExceptionFlags = EXCEPTION_NONCONTINUABLE; > + ExceptionRecord.ExceptionRecord = NULL; > + ExceptionRecord.ExceptionAddress = (PVOID)TrapFrame->Eip; > + ExceptionRecord.NumberParameters = 1; > + ExceptionRecord.ExceptionInformation[0] = TrapFrame->Ecx; > + > + KeBugCheckWithTf(KERNEL_SECURITY_CHECK_FAILURE, > + TrapFrame->Ecx, > + (ULONG_PTR)TrapFrame, > + (ULONG_PTR)&ExceptionRecord, > + 0, > + TrapFrame); > + } > +} > + > +VOID > +FASTCALL > KiGetTickCountHandler(IN PKTRAP_FRAME TrapFrame) > { > UNIMPLEMENTED_DBGBREAK(); > > >

10 years, 2 months

Re: [ros-dev] [ros-diffs] [tkreuzer] 64591: [NTOSKRNL] - Improve the random address base code in MiCreatePebOrTeb to actually make sense and not rely on retarded hacks implicitly hardcoding the PEB size in pages into the ra...

by Alex Ionescu

Where't the unit test proving your 'improved' algorithm matches XP SP2/SRV03 SP1? 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:35 2014 > New Revision: 64591 > > URL: http://svn.reactos.org/svn/reactos?rev=64591&view=rev > Log: > [NTOSKRNL] > - Improve the random address base code in MiCreatePebOrTeb to actually > make sense and not rely on retarded hacks implicitly hardcoding the PEB > size in pages into the random value generation. > > 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:35 2014 > @@ -48,13 +48,13 @@ > NTAPI > MiCreatePebOrTeb(IN PEPROCESS Process, > IN ULONG Size, > - OUT PULONG_PTR Base) > + OUT PULONG_PTR BaseAddress) > { > PETHREAD Thread = PsGetCurrentThread(); > PMMVAD_LONG Vad; > NTSTATUS Status; > - ULONG RandomCoeff; > - ULONG_PTR StartAddress, EndAddress; > + ULONG_PTR HighestAddress, RandomBase; > + ULONG AlignedSize; > LARGE_INTEGER CurrentTime; > TABLE_SEARCH_RESULT Result = TableFoundNode; > PMMADDRESS_NODE Parent; > @@ -83,25 +83,30 @@ > /* Check if this is a PEB creation */ > if (Size == sizeof(PEB)) > { > - /* Start at the highest valid address */ > - StartAddress = (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS + 1; > - > - /* Select the random coefficient */ > + /* Create a random value to select one page in a 64k region */ > KeQueryTickCount(&CurrentTime); > - CurrentTime.LowPart &= ((64 * _1KB) >> PAGE_SHIFT) - 1; > - if (CurrentTime.LowPart <= 1) CurrentTime.LowPart = 2; > - RandomCoeff = CurrentTime.LowPart << PAGE_SHIFT; > - > - /* Select the highest valid address minus the random coefficient > */ > - StartAddress -= RandomCoeff; > - EndAddress = StartAddress + ROUND_TO_PAGES(Size) - 1; > + CurrentTime.LowPart &= (_64K / PAGE_SIZE) - 1; > + > + /* Calculate a random base address */ > + RandomBase = (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS + 1; > + RandomBase -= CurrentTime.LowPart << PAGE_SHIFT; > + > + /* Make sure the base address is not too high */ > + AlignedSize = ROUND_TO_PAGES(Size); > + if ((RandomBase + AlignedSize) > > (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS + 1) > + { > + RandomBase = (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS + 1 - > AlignedSize; > + } > + > + /* Calculate the highest allowed address */ > + HighestAddress = RandomBase + AlignedSize - 1; > > /* Try to find something below the random upper margin */ > Result = MiFindEmptyAddressRangeDownTree(ROUND_TO_PAGES(Size), > - EndAddress, > + HighestAddress, > PAGE_SIZE, > &Process->VadRoot, > - Base, > + BaseAddress, > &Parent); > } > > @@ -113,7 +118,7 @@ > > (ULONG_PTR)MM_HIGHEST_VAD_ADDRESS, > PAGE_SIZE, > &Process->VadRoot, > - Base, > + BaseAddress, > &Parent); > /* Bail out, if still nothing free was found */ > if (Result == TableFoundNode) > @@ -125,12 +130,12 @@ > } > > /* Validate that it came from the VAD ranges */ > - ASSERT(*Base >= (ULONG_PTR)MI_LOWEST_VAD_ADDRESS); > + ASSERT(*BaseAddress >= (ULONG_PTR)MI_LOWEST_VAD_ADDRESS); > > /* Build the rest of the VAD now */ > - Vad->StartingVpn = (*Base) >> PAGE_SHIFT; > - Vad->EndingVpn = ((*Base) + Size - 1) >> PAGE_SHIFT; > - Vad->u3.Secured.StartVpn = *Base; > + 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; > > @@ -146,7 +151,7 @@ > 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", *Base, > Process->ImageFileName); > + DPRINT("Allocated PEB/TEB at: 0x%p for %16s\n", *BaseAddress, > Process->ImageFileName); > MiInsertNode(&Process->VadRoot, (PVOID)Vad, Parent, Result); > > /* Release the working set */ > > >

10 years, 2 months

Re: [ros-dev] [ros-diffs] [jgardou] 64537: [NTOS/MM] - Do not assert in case of stack overflow, just let the page fault handler raise STATUS_STACK_OVERFLOW

by Alex Ionescu

The ASSERT is there because of the missing functionality. Please see the comment just above. Best regards, Alex Ionescu On Sun, Oct 5, 2014 at 2:57 AM, <jgardou(a)svn.reactos.org> wrote: > Author: jgardou > Date: Sun Oct 5 09:57:02 2014 > New Revision: 64537 > > URL: http://svn.reactos.org/svn/reactos?rev=64537&view=rev > Log: > [NTOS/MM] > - Do not assert in case of stack overflow, just let the page fault > handler raise STATUS_STACK_OVERFLOW > > Modified: > trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c > > Modified: trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pagfault.… > > ============================================================================== > --- trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] Sun Oct 5 > 09:57:02 2014 > @@ -82,7 +82,6 @@ > { > /* We don't -- Windows would try to make this guard page valid > now */ > DPRINT1("Close to our death...\n"); > - ASSERT(FALSE); > return STATUS_STACK_OVERFLOW; > } > > > >

10 years, 2 months

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, 2 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev October 2014