[NTOSKRNL] Misc fixes to VACB reference counting

This fixes various bugs linked to VACB counting:
- VACB not released when it should be
- Reference count expectations not being accurate

For the record, VACB should always have at least a reference
count of 1, unless you are to free it and removed it from
any linked list.

This commit also adds a bunch of asserts that should
help triggering invalid reference counting.

It should also fix numerous ASSERT currently triggered and
may help fixing random behaviours in Cc.

CORE-14285
CORE-14401
CORE-14293

diff --git a/ntoskrnl/cc/fs.c b/ntoskrnl/cc/fs.c
index 38c9ae5..05a9a89 100644 (file)
--- a/ntoskrnl/cc/fs.c
+++ b/ntoskrnl/cc/fs.c
@@ -222,14 +222,18 @@ CcPurgeCacheSection (
             break;
         }
 
             break;
         }
 

-        /* Still in use, it cannot be purged, fail */
-        if (Vacb->ReferenceCount != 0 && !Vacb->Dirty)
+        /* Still in use, it cannot be purged, fail
+         * Allow one ref: VACB is supposed to be always 1-referenced
+         */
+        if ((Vacb->ReferenceCount > 1 && !Vacb->Dirty) ||
+            (Vacb->ReferenceCount > 2 && Vacb->Dirty))

         {
             Success = FALSE;
             break;
         }
 
         /* This VACB is in range, so unlink it and mark for free */
         {
             Success = FALSE;
             break;
         }
 
         /* This VACB is in range, so unlink it and mark for free */

+        ASSERT(Vacb->ReferenceCount == 1 || Vacb->Dirty);

         RemoveEntryList(&Vacb->VacbLruListEntry);
         if (Vacb->Dirty)
         {
         RemoveEntryList(&Vacb->VacbLruListEntry);
         if (Vacb->Dirty)
         {


@@ -246,6 +250,7 @@ CcPurgeCacheSection (
         Vacb = CONTAINING_RECORD(RemoveHeadList(&FreeList),
                                  ROS_VACB,
                                  CacheMapVacbListEntry);
         Vacb = CONTAINING_RECORD(RemoveHeadList(&FreeList),
                                  ROS_VACB,
                                  CacheMapVacbListEntry);

+        CcRosVacbDecRefCount(Vacb);

         CcRosInternalFreeVacb(Vacb);
     }
 
         CcRosInternalFreeVacb(Vacb);
     }
 

diff --git a/ntoskrnl/cc/pin.c b/ntoskrnl/cc/pin.c
index 20b4096..c916783 100644 (file)
--- a/ntoskrnl/cc/pin.c
+++ b/ntoskrnl/cc/pin.c
@@ -382,7 +382,15 @@ CcUnpinRepinnedBcb (
             iBcb->Pinned = FALSE;
             CcRosAcquireVacbLock(iBcb->Vacb, NULL);
             iBcb->Vacb->PinCount--;
             iBcb->Pinned = FALSE;
             CcRosAcquireVacbLock(iBcb->Vacb, NULL);
             iBcb->Vacb->PinCount--;

+            ASSERT(iBcb->Vacb->PinCount == 0);

         }
         }

+
+        CcRosReleaseVacb(iBcb->Vacb->SharedCacheMap,
+                         iBcb->Vacb,
+                         TRUE,
+                         iBcb->Dirty,
+                         FALSE);
+

         ExDeleteResourceLite(&iBcb->Lock);
         ExFreeToNPagedLookasideList(&iBcbLookasideList, iBcb);
     }
         ExDeleteResourceLite(&iBcb->Lock);
         ExFreeToNPagedLookasideList(&iBcbLookasideList, iBcb);
     }

diff --git a/ntoskrnl/cc/view.c b/ntoskrnl/cc/view.c
index cf2d2c5..4651abe 100644 (file)
--- a/ntoskrnl/cc/view.c
+++ b/ntoskrnl/cc/view.c
@@ -65,7 +65,7 @@ KSPIN_LOCK CcDeferredWriteSpinLock;
 LIST_ENTRY CcCleanSharedCacheMapList;
 
 #if DBG
 LIST_ENTRY CcCleanSharedCacheMapList;
 
 #if DBG

-static void CcRosVacbIncRefCount_(PROS_VACB vacb, const char* file, int line)
+VOID CcRosVacbIncRefCount_(PROS_VACB vacb, PCSTR file, INT line)

 {
     ++vacb->ReferenceCount;
     if (vacb->SharedCacheMap->Trace)
 {
     ++vacb->ReferenceCount;
     if (vacb->SharedCacheMap->Trace)


@@ -74,7 +74,7 @@ static void CcRosVacbIncRefCount_(PROS_VACB vacb, const char* file, int line)
                  file, line, vacb, vacb->ReferenceCount, vacb->Dirty, vacb->PageOut);
     }
 }
                  file, line, vacb, vacb->ReferenceCount, vacb->Dirty, vacb->PageOut);
     }
 }

-static void CcRosVacbDecRefCount_(PROS_VACB vacb, const char* file, int line)
+VOID CcRosVacbDecRefCount_(PROS_VACB vacb, PCSTR file, INT line)

 {
     ASSERT(vacb->ReferenceCount != 0);
     --vacb->ReferenceCount;
 {
     ASSERT(vacb->ReferenceCount != 0);
     --vacb->ReferenceCount;


@@ -85,11 +85,6 @@ static void CcRosVacbDecRefCount_(PROS_VACB vacb, const char* file, int line)
                  file, line, vacb, vacb->ReferenceCount, vacb->Dirty, vacb->PageOut);
     }
 }
                  file, line, vacb, vacb->ReferenceCount, vacb->Dirty, vacb->PageOut);
     }
 }

-#define CcRosVacbIncRefCount(vacb) CcRosVacbIncRefCount_(vacb,__FILE__,__LINE__)
-#define CcRosVacbDecRefCount(vacb) CcRosVacbDecRefCount_(vacb,__FILE__,__LINE__)
-#else
-#define CcRosVacbIncRefCount(vacb) (++((vacb)->ReferenceCount))
-#define CcRosVacbDecRefCount(vacb) (--((vacb)->ReferenceCount))

 #endif
 
 NTSTATUS
 #endif
 
 NTSTATUS


@@ -350,10 +345,11 @@ retry:
         CcRosVacbDecRefCount(current);
 
         /* Check if we can free this entry now */
         CcRosVacbDecRefCount(current);
 
         /* Check if we can free this entry now */

-        if (current->ReferenceCount == 0)
+        if (current->ReferenceCount < 2)

         {
             ASSERT(!current->Dirty);
             ASSERT(!current->MappedCount);
         {
             ASSERT(!current->Dirty);
             ASSERT(!current->MappedCount);

+            ASSERT(current->ReferenceCount == 1);

 
             RemoveEntryList(&current->CacheMapVacbListEntry);
             RemoveEntryList(&current->VacbLruListEntry);
 
             RemoveEntryList(&current->CacheMapVacbListEntry);
             RemoveEntryList(&current->VacbLruListEntry);


@@ -395,6 +391,7 @@ retry:
         current = CONTAINING_RECORD(current_entry,
                                     ROS_VACB,
                                     CacheMapVacbListEntry);
         current = CONTAINING_RECORD(current_entry,
                                     ROS_VACB,
                                     CacheMapVacbListEntry);

+        CcRosVacbDecRefCount(current);

         CcRosInternalFreeVacb(current);
     }
 
         CcRosInternalFreeVacb(current);
     }
 


@@ -434,6 +431,8 @@ CcRosReleaseVacb (
         CcRosVacbIncRefCount(Vacb);
     }
 
         CcRosVacbIncRefCount(Vacb);
     }
 

+    ASSERT(Vacb->ReferenceCount != 0);
+

     CcRosReleaseVacbLock(Vacb);
 
     return STATUS_SUCCESS;
     CcRosReleaseVacbLock(Vacb);
 
     return STATUS_SUCCESS;


@@ -575,16 +574,15 @@ CcRosMarkDirtyFile (
         KeBugCheck(CACHE_MANAGER);
     }
 
         KeBugCheck(CACHE_MANAGER);
     }
 

-    if (!Vacb->Dirty)
-    {
-        CcRosMarkDirtyVacb(Vacb);
-    }
-
-    CcRosReleaseVacbLock(Vacb);
+    CcRosReleaseVacb(SharedCacheMap, Vacb, Vacb->Valid, TRUE, FALSE);

 
     return STATUS_SUCCESS;
 }
 
 
     return STATUS_SUCCESS;
 }
 

+/*
+ * Note: this is not the contrary function of
+ * CcRosMapVacbInKernelSpace()
+ */

 NTSTATUS
 NTAPI
 CcRosUnmapVacb (
 NTSTATUS
 NTAPI
 CcRosUnmapVacb (


@@ -605,28 +603,22 @@ CcRosUnmapVacb (
         return STATUS_UNSUCCESSFUL;
     }
 
         return STATUS_UNSUCCESSFUL;
     }
 

-    if (NowDirty && !Vacb->Dirty)
-    {
-        CcRosMarkDirtyVacb(Vacb);
-    }
-

     ASSERT(Vacb->MappedCount != 0);
     Vacb->MappedCount--;
 
     ASSERT(Vacb->MappedCount != 0);
     Vacb->MappedCount--;
 

-    CcRosVacbDecRefCount(Vacb);

     if (Vacb->MappedCount == 0)
     {
         CcRosVacbDecRefCount(Vacb);
     }
 
     if (Vacb->MappedCount == 0)
     {
         CcRosVacbDecRefCount(Vacb);
     }
 

-    CcRosReleaseVacbLock(Vacb);
+    CcRosReleaseVacb(SharedCacheMap, Vacb, Vacb->Valid, NowDirty, FALSE);

 
     return STATUS_SUCCESS;
 }
 
 static
 NTSTATUS
 
     return STATUS_SUCCESS;
 }
 
 static
 NTSTATUS

-CcRosMapVacb(
+CcRosMapVacbInKernelSpace(

     PROS_VACB Vacb)
 {
     ULONG i;
     PROS_VACB Vacb)
 {
     ULONG i;


@@ -721,7 +713,7 @@ CcRosCreateVacb (
     current->MappedCount = 0;
     current->DirtyVacbListEntry.Flink = NULL;
     current->DirtyVacbListEntry.Blink = NULL;
     current->MappedCount = 0;
     current->DirtyVacbListEntry.Flink = NULL;
     current->DirtyVacbListEntry.Blink = NULL;

-    current->ReferenceCount = 1;
+    current->ReferenceCount = 0;

     current->PinCount = 0;
     KeInitializeMutex(&current->Mutex, 0);
     CcRosAcquireVacbLock(current, NULL);
     current->PinCount = 0;
     KeInitializeMutex(&current->Mutex, 0);
     CcRosAcquireVacbLock(current, NULL);


@@ -785,6 +777,7 @@ CcRosCreateVacb (
     }
     KeReleaseSpinLock(&SharedCacheMap->CacheMapLock, oldIrql);
     InsertTailList(&VacbLruListHead, &current->VacbLruListEntry);
     }
     KeReleaseSpinLock(&SharedCacheMap->CacheMapLock, oldIrql);
     InsertTailList(&VacbLruListHead, &current->VacbLruListEntry);

+    CcRosVacbIncRefCount(current);

     KeReleaseGuardedMutex(&ViewLock);
 
     MI_SET_USAGE(MI_USAGE_CACHE);
     KeReleaseGuardedMutex(&ViewLock);
 
     MI_SET_USAGE(MI_USAGE_CACHE);


@@ -806,7 +799,7 @@ CcRosCreateVacb (
     }
 #endif
 
     }
 #endif
 

-    Status = CcRosMapVacb(current);
+    Status = CcRosMapVacbInKernelSpace(current);

     if (!NT_SUCCESS(Status))
     {
         RemoveEntryList(&current->CacheMapVacbListEntry);
     if (!NT_SUCCESS(Status))
     {
         RemoveEntryList(&current->CacheMapVacbListEntry);


@@ -849,6 +842,8 @@ CcRosGetVacb (
         {
             return Status;
         }
         {
             return Status;
         }

+
+        CcRosVacbIncRefCount(current);

     }
 
     KeAcquireGuardedMutex(&ViewLock);
     }
 
     KeAcquireGuardedMutex(&ViewLock);


@@ -941,6 +936,13 @@ CcRosInternalFreeVacb (
                      NULL);
     MmUnlockAddressSpace(MmGetKernelAddressSpace());
 
                      NULL);
     MmUnlockAddressSpace(MmGetKernelAddressSpace());
 

+    if (Vacb->PinCount != 0 || Vacb->ReferenceCount != 0)
+    {
+        DPRINT1("Invalid free: %ld, %ld\n", Vacb->ReferenceCount, Vacb->PinCount);
+    }
+
+    ASSERT(Vacb->PinCount == 0);
+    ASSERT(Vacb->ReferenceCount == 0);

     ExFreeToNPagedLookasideList(&VacbLookasideList, Vacb);
     return STATUS_SUCCESS;
 }
     ExFreeToNPagedLookasideList(&VacbLookasideList, Vacb);
     return STATUS_SUCCESS;
 }


@@ -1092,6 +1094,7 @@ CcRosDeleteFileCache (
         {
             current_entry = RemoveTailList(&FreeList);
             current = CONTAINING_RECORD(current_entry, ROS_VACB, CacheMapVacbListEntry);
         {
             current_entry = RemoveTailList(&FreeList);
             current = CONTAINING_RECORD(current_entry, ROS_VACB, CacheMapVacbListEntry);

+            CcRosVacbDecRefCount(current);

             CcRosInternalFreeVacb(current);
         }
 
             CcRosInternalFreeVacb(current);
         }
 

diff --git a/ntoskrnl/include/internal/cc.h b/ntoskrnl/include/internal/cc.h
index e91f1b4..ad939a1 100644 (file)
--- a/ntoskrnl/include/internal/cc.h
+++ b/ntoskrnl/include/internal/cc.h
@@ -519,3 +519,24 @@ IsPointInRange(
 }
 
 #define CcBugCheck(A, B, C) KeBugCheckEx(CACHE_MANAGER, BugCheckFileId | ((ULONG)(__LINE__)), A, B, C)
 }
 
 #define CcBugCheck(A, B, C) KeBugCheckEx(CACHE_MANAGER, BugCheckFileId | ((ULONG)(__LINE__)), A, B, C)

+
+#if DBG
+#define CcRosVacbIncRefCount(vacb) CcRosVacbIncRefCount_(vacb,__FILE__,__LINE__)
+#define CcRosVacbDecRefCount(vacb) CcRosVacbDecRefCount_(vacb,__FILE__,__LINE__)
+
+VOID
+CcRosVacbIncRefCount_(
+    PROS_VACB vacb,
+    PCSTR file,
+    INT line);
+
+VOID
+CcRosVacbDecRefCount_(
+    PROS_VACB vacb,
+    PCSTR file,
+    INT line);
+
+#else
+#define CcRosVacbIncRefCount(vacb) (++((vacb)->ReferenceCount))
+#define CcRosVacbDecRefCount(vacb) (--((vacb)->ReferenceCount))
+#endif