Author: sir_richard
Date: Sat Jul 24 16:28:51 2010
New Revision: 48235
URL: http://svn.reactos.org/svn/reactos?rev=48235&view=rev
Log:
[NTOS]: Implement MmDeleteTeb, VADs are now deleted/freed on thread exit as well (but the underlying page is still leaked). Should fix the advapi32 security crash.
[NTOS]: Sometimes it seems we hit some bad VADs due to bugs? in the AVL tree implementation. I'm going on vacation for a month and can't look at this, so I've hacked the code to ignore such VADs for now, in the interest of fixing the winetest regression.
Modified:
trunk/reactos/ntoskrnl/mm/ARM3/miarm.h
trunk/reactos/ntoskrnl/mm/ARM3/procsup.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] Sat Jul 24 16:28:51 2010
@@ -1121,4 +1121,16 @@
IN PMM_AVL_TABLE Table
);
+PMMADDRESS_NODE
+NTAPI
+MiGetPreviousNode(
+ IN PMMADDRESS_NODE Node
+);
+
+PMMADDRESS_NODE
+NTAPI
+MiGetNextNode(
+ IN PMMADDRESS_NODE Node
+);
+
/* EOF */
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] Sat Jul 24 16:28:51 2010
@@ -149,8 +149,57 @@
MmDeleteTeb(IN PEPROCESS Process,
IN PTEB Teb)
{
- /* Oops J */
- DPRINT("Leaking 4KB at thread exit, this will be fixed later\n");
+ ULONG_PTR TebEnd;
+ PETHREAD Thread = PsGetCurrentThread();
+ PMMVAD Vad;
+ PMM_AVL_TABLE VadTree = &Process->VadRoot;
+ DPRINT("Deleting TEB: %p in %16s\n", Teb, Process->ImageFileName);
+
+ /* TEB is one page */
+ TebEnd = (ULONG_PTR)Teb + ROUND_TO_PAGES(sizeof(TEB)) - 1;
+
+ /* Attach to the process */
+ KeAttachProcess(&Process->Pcb);
+
+ /* Lock the process address space */
+ KeAcquireGuardedMutex(&Process->AddressCreationLock);
+
+ /* Find the VAD, make sure it's a TEB VAD */
+ Vad = MiLocateAddress(Teb);
+ DPRINT("Removing node for VAD: %lx %lx\n", Vad->StartingVpn, Vad->EndingVpn);
+ ASSERT(Vad != NULL);
+ if (Vad->StartingVpn != ((ULONG_PTR)Teb >> PAGE_SHIFT))
+ {
+ /* Bug in the AVL code? */
+ DPRINT1("Corrupted VAD!\n");
+ }
+ else
+ {
+ /* Sanity checks for a valid TEB VAD */
+ ASSERT((Vad->StartingVpn == ((ULONG_PTR)Teb >> PAGE_SHIFT) &&
+ (Vad->EndingVpn == (TebEnd >> PAGE_SHIFT))));
+ ASSERT(Vad->u.VadFlags.NoChange == TRUE);
+ ASSERT(Vad->u2.VadFlags2.MultipleSecured == FALSE);
+
+ /* Lock the working set */
+ MiLockProcessWorkingSet(Process, Thread);
+
+ /* Remove this VAD from the tree */
+ ASSERT(VadTree->NumberGenericTableElements >= 1);
+ MiRemoveNode((PMMADDRESS_NODE)Vad, VadTree);
+
+ /* Release the working set */
+ MiUnlockProcessWorkingSet(Process, Thread);
+
+ /* Remove the VAD */
+ ExFreePool(Vad);
+ }
+
+ /* Release the address space lock */
+ KeReleaseGuardedMutex(&Process->AddressCreationLock);
+
+ /* Detach */
+ KeDetachProcess();
}
VOID
Author: sir_richard
Date: Sat Jul 24 16:12:39 2010
New Revision: 48234
URL: http://svn.reactos.org/svn/reactos?rev=48234&view=rev
Log:
[NTOS]: Implement MmCleanProcessAddressSpace in ARM3, now the PEB/TEB VADs are removed when the process exits (although the pages are still leaking, for now), and the pool allocation for the VAD is also freed.
[NTOS]: Use ARM3 paged pool up until smss.exe starts. There's a last bug in the expansion code before we can get rid of the old paged pool.
Modified:
trunk/reactos/lib/rtl/avlsupp.c
trunk/reactos/ntoskrnl/ex/init.c
trunk/reactos/ntoskrnl/mm/ARM3/procsup.c
trunk/reactos/ntoskrnl/mm/mminit.c
trunk/reactos/ntoskrnl/mm/procsup.c
Modified: trunk/reactos/lib/rtl/avlsupp.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/avlsupp.c?rev=4823…
==============================================================================
--- trunk/reactos/lib/rtl/avlsupp.c [iso-8859-1] (original)
+++ trunk/reactos/lib/rtl/avlsupp.c [iso-8859-1] Sat Jul 24 16:12:39 2010
@@ -297,7 +297,7 @@
RtlpDeleteAvlTreeNode(IN PRTL_AVL_TABLE Table,
IN PRTL_BALANCED_LINKS Node)
{
- PRTL_BALANCED_LINKS DeleteNode, ParentNode;
+ PRTL_BALANCED_LINKS DeleteNode = NULL, ParentNode;
PRTL_BALANCED_LINKS *Node1, *Node2;
CHAR Balance;
@@ -320,15 +320,19 @@
/* Get the parent node */
ParentNode = RtlParentAvl(DeleteNode);
+ DPRINT("Parent: %p\n", ParentNode);
/* Pick which now to use based on whether or not we have a left child */
Node1 = RtlLeftChildAvl(DeleteNode) ? &DeleteNode->LeftChild : &DeleteNode->RightChild;
+ DPRINT("Node 1: %p %p\n", Node1, *Node1);
/* Pick which node to swap based on if we're already a left child or not */
Node2 = RtlIsLeftChildAvl(DeleteNode) ? &ParentNode->LeftChild : &ParentNode->RightChild;
-
+ DPRINT("Node 2: %p %p\n", Node2, *Node2);
+
/* Pick the correct balance depending on which side will get heavier */
Balance = RtlIsLeftChildAvl(DeleteNode) ? RtlLeftHeavyAvlTree : RtlRightHeavyAvlTree;
+ DPRINT("Balance: %lx\n", Balance);
/* Swap the children nodes, making one side heavier */
*Node2 = *Node1;
Modified: trunk/reactos/ntoskrnl/ex/init.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/init.c?rev=482…
==============================================================================
--- trunk/reactos/ntoskrnl/ex/init.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ex/init.c [iso-8859-1] Sat Jul 24 16:12:39 2010
@@ -1859,6 +1859,10 @@
/* Allow strings to be displayed */
InbvEnableDisplayString(TRUE);
+ /* Enough fun for now */
+ extern BOOLEAN AllowPagedPool;
+ AllowPagedPool = FALSE;
+
/* Wait 5 seconds for it to initialize */
Timeout.QuadPart = Int32x32To64(5, -10000000);
Status = ZwWaitForSingleObject(ProcessInfo->ProcessHandle, FALSE, &Timeout);
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] Sat Jul 24 16:12:39 2010
@@ -1083,6 +1083,57 @@
return TRUE;
}
+VOID
+NTAPI
+MmCleanProcessAddressSpace(IN PEPROCESS Process)
+{
+ PMMVAD Vad;
+ PMM_AVL_TABLE VadTree;
+ PETHREAD Thread = PsGetCurrentThread();
+
+ /* Lock the process address space from changes */
+ MmLockAddressSpace(&Process->Vm);
+
+ /* Enumerate the VADs */
+ VadTree = &Process->VadRoot;
+ DPRINT("Cleaning up VADs: %d\n", VadTree->NumberGenericTableElements);
+ while (VadTree->NumberGenericTableElements)
+ {
+ /* Grab the current VAD */
+ Vad = (PMMVAD)VadTree->BalancedRoot.RightChild;
+
+ /* Lock the working set */
+ MiLockProcessWorkingSet(Process, Thread);
+
+ /* Remove this VAD from the tree */
+ ASSERT(VadTree->NumberGenericTableElements >= 1);
+ DPRINT("Removing node for VAD: %lx %lx\n", Vad->StartingVpn, Vad->EndingVpn);
+ MiRemoveNode((PMMADDRESS_NODE)Vad, VadTree);
+ DPRINT("Moving on: %d\n", VadTree->NumberGenericTableElements);
+
+ /* Check if this VAD was the hint */
+ if (VadTree->NodeHint == Vad)
+ {
+ /* Get a new hint, unless we're empty now, in which case nothing */
+ VadTree->NodeHint = VadTree->BalancedRoot.RightChild;
+ if (!VadTree->NumberGenericTableElements) VadTree->NodeHint = NULL;
+ }
+
+ /* Only PEB/TEB VADs supported for now */
+ ASSERT(Vad->u.VadFlags.PrivateMemory == 1);
+ ASSERT(Vad->u.VadFlags.VadType == VadNone);
+
+ /* Release the working set */
+ MiUnlockProcessWorkingSet(Process, Thread);
+
+ /* Free the VAD memory */
+ ExFreePool(Vad);
+ }
+
+ /* Release the address space */
+ MmUnlockAddressSpace(&Process->Vm);
+}
+
/* SYSTEM CALLS ***************************************************************/
NTSTATUS
Modified: trunk/reactos/ntoskrnl/mm/mminit.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/mminit.c?rev=4…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/mminit.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/mminit.c [iso-8859-1] Sat Jul 24 16:12:39 2010
@@ -436,12 +436,6 @@
/* Initialize the balance set manager */
MmInitBsmThread();
}
- else if (Phase == 2)
- {
- /* Enough fun for now */
- extern BOOLEAN AllowPagedPool;
- AllowPagedPool = FALSE;
- }
return TRUE;
}
Modified: trunk/reactos/ntoskrnl/mm/procsup.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/procsup.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/procsup.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/procsup.c [iso-8859-1] Sat Jul 24 16:12:39 2010
@@ -14,13 +14,6 @@
#include <debug.h>
/* FUNCTIONS *****************************************************************/
-
-VOID
-NTAPI
-MmCleanProcessAddressSpace(IN PEPROCESS Process)
-{
- /* FIXME: Add part of MmDeleteProcessAddressSpace here */
-}
NTSTATUS
NTAPI