[ros-diffs] 01/01: [NTOS:CC] Access SectionObjectPointers without lock in CcRosInitializeFileCache. CORE-14691

[Thomas Faber]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=1d398057a3394298d2b25…

commit 1d398057a3394298d2b251862f383d6eb27142f1
Author:     Thomas Faber <thomas.faber(a)reactos.org>
AuthorDate: Mon Jun 4 14:36:07 2018 +0200
Commit:     Thomas Faber <thomas.faber(a)reactos.org>
CommitDate: Tue Jun 5 16:24:13 2018 +0200

    [NTOS:CC] Access SectionObjectPointers without lock in CcRosInitializeFileCache.
CORE-14691
    
    kmtest:NtCreateSection calls CcInitializeCacheMap with a
    NULL value for SectionObjectPointers. This will cause an exception when
    trying to access it, which in Windows can be handled gracefully.
    However accessing it while holding ViewLock means the lock will not be
    released, leading to an APC_INDEX_MISMATCH bugcheck.
    
    This solves the problem by allocating SharedCacheMap outside the lock,
    then freeing it again under lock if another thread has updated SharedCacheMap
    in the mean time. This is also What Windows Does(TM).
---
 ntoskrnl/cc/view.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/ntoskrnl/cc/view.c b/ntoskrnl/cc/view.c
index 1909c8bb1e..6a087e6531 100644
--- a/ntoskrnl/cc/view.c
+++ b/ntoskrnl/cc/view.c
@@ -1394,7 +1394,6 @@ CcRosInitializeFileCache (
     DPRINT("CcRosInitializeFileCache(FileObject 0x%p)\n", FileObject);
 
     Allocated = FALSE;
-    KeAcquireGuardedMutex(&ViewLock);
     SharedCacheMap = FileObject->SectionObjectPointer->SharedCacheMap;
     if (SharedCacheMap == NULL)
     {
@@ -1402,14 +1401,9 @@ CcRosInitializeFileCache (
         SharedCacheMap =
ExAllocateFromNPagedLookasideList(&SharedCacheMapLookasideList);
         if (SharedCacheMap == NULL)
         {
-            KeReleaseGuardedMutex(&ViewLock);
             return STATUS_INSUFFICIENT_RESOURCES;
         }
         RtlZeroMemory(SharedCacheMap, sizeof(*SharedCacheMap));
-        ObReferenceObjectByPointer(FileObject,
-                                   FILE_ALL_ACCESS,
-                                   NULL,
-                                   KernelMode);
         SharedCacheMap->NodeTypeCode = NODE_TYPE_SHARED_MAP;
         SharedCacheMap->NodeByteSize = sizeof(*SharedCacheMap);
         SharedCacheMap->FileObject = FileObject;
@@ -1423,11 +1417,28 @@ CcRosInitializeFileCache (
         InitializeListHead(&SharedCacheMap->PrivateList);
         KeInitializeSpinLock(&SharedCacheMap->CacheMapLock);
         InitializeListHead(&SharedCacheMap->CacheMapVacbListHead);
-        FileObject->SectionObjectPointer->SharedCacheMap = SharedCacheMap;
+    }
 
-        OldIrql = KeAcquireQueuedSpinLock(LockQueueMasterLock);
-        InsertTailList(&CcCleanSharedCacheMapList,
&SharedCacheMap->SharedCacheMapLinks);
-        KeReleaseQueuedSpinLock(LockQueueMasterLock, OldIrql);
+    KeAcquireGuardedMutex(&ViewLock);
+    if (Allocated)
+    {
+        if (FileObject->SectionObjectPointer->SharedCacheMap == NULL)
+        {
+            ObReferenceObjectByPointer(FileObject,
+                                       FILE_ALL_ACCESS,
+                                       NULL,
+                                       KernelMode);
+            FileObject->SectionObjectPointer->SharedCacheMap = SharedCacheMap;
+
+            OldIrql = KeAcquireQueuedSpinLock(LockQueueMasterLock);
+            InsertTailList(&CcCleanSharedCacheMapList,
&SharedCacheMap->SharedCacheMapLinks);
+            KeReleaseQueuedSpinLock(LockQueueMasterLock, OldIrql);
+        }
+        else
+        {
+            ExFreeToNPagedLookasideList(&SharedCacheMapLookasideList,
SharedCacheMap);
+            SharedCacheMap = FileObject->SectionObjectPointer->SharedCacheMap;
+        }
     }
     if (FileObject->PrivateCacheMap == NULL)
     {