[rmessiant] 52002: [NTOSKRNL] - Simplify remove lock tracking block list handling. - Correct check for unlimited locking time being allowed. - Eliminate use-after-free after removing a tracking block.

Author: rmessiant
Date: Sun May 29 22:29:10 2011
New Revision: 52002

URL: http://svn.reactos.org/svn/reactos?rev=52002&view=rev
Log:
[NTOSKRNL]
- Simplify remove lock tracking block list handling.
- Correct check for unlimited locking time being allowed.
- Eliminate use-after-free after removing a tracking block.

Modified:
    trunk/reactos/ntoskrnl/io/iomgr/remlock.c

Modified: trunk/reactos/ntoskrnl/io/iomgr/remlock.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/remlock.c...
==============================================================================
--- trunk/reactos/ntoskrnl/io/iomgr/remlock.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/io/iomgr/remlock.c [iso-8859-1] Sun May 29 22:29:10 2011
@@ -16,7 +16,7 @@

typedef struct _IO_REMOVE_LOCK_TRACKING_BLOCK
 {
-    SINGLE_LIST_ENTRY BlockEntry;
+    PIO_REMOVE_LOCK_TRACKING_BLOCK Next;
     PVOID Tag;
     LARGE_INTEGER LockMoment;
     LPCSTR File;
@@ -114,7 +114,8 @@

/* Queue the block */
                 KeAcquireSpinLock(&(Lock->Dbg.Spin), &OldIrql);
-                PushEntryList((PSINGLE_LIST_ENTRY)&(Lock->Dbg.Blocks), &(TrackingBlock->BlockEntry));
+                TrackingBlock->Next = Lock->Dbg.Blocks;
+                Lock->Dbg.Blocks = TrackingBlock;
                 KeReleaseSpinLock(&(Lock->Dbg.Spin), OldIrql);
             }
         }
@@ -149,8 +150,8 @@
     LONG LockValue;
     BOOLEAN TagFound;
     LARGE_INTEGER CurrentMoment;
-    PSINGLE_LIST_ENTRY ListEntry;
     PIO_REMOVE_LOCK_TRACKING_BLOCK TrackingBlock;
+    PIO_REMOVE_LOCK_TRACKING_BLOCK *TrackingBlockLink;
     PEXTENDED_IO_REMOVE_LOCK Lock = (PEXTENDED_IO_REMOVE_LOCK)RemoveLock;

/* Check what kind of lock this is */
@@ -164,12 +165,12 @@

/* Start browsing tracking blocks to find a block that would match given tag */
         TagFound = FALSE;
-        for (ListEntry = ((PSINGLE_LIST_ENTRY)&Lock->Dbg.Blocks)->Next; ListEntry; ListEntry = ListEntry->Next)
-        {
-            TrackingBlock = CONTAINING_RECORD(ListEntry, IO_REMOVE_LOCK_TRACKING_BLOCK, BlockEntry);
-
+        TrackingBlock = Lock->Dbg.Blocks;
+        TrackingBlockLink = &(Lock->Dbg.Blocks);
+        while (TrackingBlock != NULL)
+        {
             /* First of all, check if the lock was locked for too long */
-            if (CurrentMoment.QuadPart &&
+            if (TrackingBlock->LockMoment.QuadPart &&
                 CurrentMoment.QuadPart - TrackingBlock->LockMoment.QuadPart >  Lock->Dbg.MaxLockedTicks)
             {
                 DPRINT("Lock %#08lx (with tag %#08lx) was supposed to be held at max %I64d ticks but lasted longer\n",
@@ -178,29 +179,23 @@
                 ASSERT(FALSE);
             }

-            /* If no tracking was found yet */
-            if (TagFound == FALSE)
-            {
-                /* Check if the current one could match */
-                if (TrackingBlock->Tag == Tag)
-                {
-                    /* Yes, then remove it from the queue and free it */
-                    TagFound = TRUE;
-                    if (ListEntry == ((PSINGLE_LIST_ENTRY)&Lock->Dbg.Blocks)->Next)
-                    {
-                        /* Here it is head, remove it using macro */
-                        PopEntryList((PSINGLE_LIST_ENTRY)&(Lock->Dbg.Blocks));
-                        ExFreePoolWithTag(TrackingBlock, Lock->Dbg.AllocateTag);
-                    }
-                    else
-                    {
-                        /* It's not head, remove it "manually */
-                        ListEntry->Next = TrackingBlock->BlockEntry.Next;
-                        ExFreePoolWithTag(TrackingBlock, Lock->Dbg.AllocateTag);
-                    }
-                }
-             }
-        }
+            /* Check if this is the first matching tracking block */
+            if ((TagFound == FALSE) && (TrackingBlock->Tag == Tag))
+            {
+                /* Unlink this tracking block, and free it */
+                TagFound = TRUE;
+                *TrackingBlockLink = TrackingBlock->Next;
+                ExFreePoolWithTag(TrackingBlock, Lock->Dbg.AllocateTag);
+                TrackingBlock = *TrackingBlockLink;
+            }
+            else
+            {
+                /* Go to the next tracking block */
+                TrackingBlockLink = &(TrackingBlock->Next);
+                TrackingBlock = TrackingBlock->Next;
+            }
+        }
+
         /* We're done, release queue lock */
         KeReleaseSpinLock(&(Lock->Dbg.Spin), OldIrql);

@@ -271,7 +266,7 @@
         ASSERT(Lock->Dbg.Blocks);

/* Get it */
-        TrackingBlock = CONTAINING_RECORD(Lock->Dbg.Blocks, IO_REMOVE_LOCK_TRACKING_BLOCK, BlockEntry);
+        TrackingBlock = Lock->Dbg.Blocks;
         /* Tag should match */
         if (TrackingBlock->Tag != Tag)
         {