[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
This commit is contained in:
Pierre Schweitzer 2018-03-17 11:56:25 +01:00
parent ad3bda1c8d
commit 13b57fb5b5
No known key found for this signature in database
GPG key ID: 7545556C3D585B0B
4 changed files with 63 additions and 26 deletions

View file

@ -222,14 +222,18 @@ CcPurgeCacheSection (
break; break;
} }
/* Still in use, it cannot be purged, fail */ /* Still in use, it cannot be purged, fail
if (Vacb->ReferenceCount != 0 && !Vacb->Dirty) * Allow one ref: VACB is supposed to be always 1-referenced
*/
if ((Vacb->ReferenceCount > 1 && !Vacb->Dirty) ||
(Vacb->ReferenceCount > 2 && Vacb->Dirty))
{ {
Success = FALSE; Success = FALSE;
break; break;
} }
/* This VACB is in range, so unlink it and mark for free */ /* This VACB is in range, so unlink it and mark for free */
ASSERT(Vacb->ReferenceCount == 1 || Vacb->Dirty);
RemoveEntryList(&Vacb->VacbLruListEntry); RemoveEntryList(&Vacb->VacbLruListEntry);
if (Vacb->Dirty) if (Vacb->Dirty)
{ {
@ -246,6 +250,7 @@ CcPurgeCacheSection (
Vacb = CONTAINING_RECORD(RemoveHeadList(&FreeList), Vacb = CONTAINING_RECORD(RemoveHeadList(&FreeList),
ROS_VACB, ROS_VACB,
CacheMapVacbListEntry); CacheMapVacbListEntry);
CcRosVacbDecRefCount(Vacb);
CcRosInternalFreeVacb(Vacb); CcRosInternalFreeVacb(Vacb);
} }

View file

@ -382,7 +382,15 @@ CcUnpinRepinnedBcb (
iBcb->Pinned = FALSE; iBcb->Pinned = FALSE;
CcRosAcquireVacbLock(iBcb->Vacb, NULL); CcRosAcquireVacbLock(iBcb->Vacb, NULL);
iBcb->Vacb->PinCount--; iBcb->Vacb->PinCount--;
ASSERT(iBcb->Vacb->PinCount == 0);
} }
CcRosReleaseVacb(iBcb->Vacb->SharedCacheMap,
iBcb->Vacb,
TRUE,
iBcb->Dirty,
FALSE);
ExDeleteResourceLite(&iBcb->Lock); ExDeleteResourceLite(&iBcb->Lock);
ExFreeToNPagedLookasideList(&iBcbLookasideList, iBcb); ExFreeToNPagedLookasideList(&iBcbLookasideList, iBcb);
} }

View file

@ -65,7 +65,7 @@ KSPIN_LOCK CcDeferredWriteSpinLock;
LIST_ENTRY CcCleanSharedCacheMapList; LIST_ENTRY CcCleanSharedCacheMapList;
#if DBG #if DBG
static void CcRosVacbIncRefCount_(PROS_VACB vacb, const char* file, int line) VOID CcRosVacbIncRefCount_(PROS_VACB vacb, PCSTR file, INT line)
{ {
++vacb->ReferenceCount; ++vacb->ReferenceCount;
if (vacb->SharedCacheMap->Trace) 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); ASSERT(vacb->ReferenceCount != 0);
--vacb->ReferenceCount; --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 #endif
NTSTATUS NTSTATUS
@ -350,10 +345,11 @@ retry:
CcRosVacbDecRefCount(current); CcRosVacbDecRefCount(current);
/* Check if we can free this entry now */ /* Check if we can free this entry now */
if (current->ReferenceCount == 0) if (current->ReferenceCount < 2)
{ {
ASSERT(!current->Dirty); ASSERT(!current->Dirty);
ASSERT(!current->MappedCount); ASSERT(!current->MappedCount);
ASSERT(current->ReferenceCount == 1);
RemoveEntryList(&current->CacheMapVacbListEntry); RemoveEntryList(&current->CacheMapVacbListEntry);
RemoveEntryList(&current->VacbLruListEntry); RemoveEntryList(&current->VacbLruListEntry);
@ -395,6 +391,7 @@ retry:
current = CONTAINING_RECORD(current_entry, current = CONTAINING_RECORD(current_entry,
ROS_VACB, ROS_VACB,
CacheMapVacbListEntry); CacheMapVacbListEntry);
CcRosVacbDecRefCount(current);
CcRosInternalFreeVacb(current); CcRosInternalFreeVacb(current);
} }
@ -434,6 +431,8 @@ CcRosReleaseVacb (
CcRosVacbIncRefCount(Vacb); CcRosVacbIncRefCount(Vacb);
} }
ASSERT(Vacb->ReferenceCount != 0);
CcRosReleaseVacbLock(Vacb); CcRosReleaseVacbLock(Vacb);
return STATUS_SUCCESS; return STATUS_SUCCESS;
@ -575,16 +574,15 @@ CcRosMarkDirtyFile (
KeBugCheck(CACHE_MANAGER); KeBugCheck(CACHE_MANAGER);
} }
if (!Vacb->Dirty) CcRosReleaseVacb(SharedCacheMap, Vacb, Vacb->Valid, TRUE, FALSE);
{
CcRosMarkDirtyVacb(Vacb);
}
CcRosReleaseVacbLock(Vacb);
return STATUS_SUCCESS; return STATUS_SUCCESS;
} }
/*
* Note: this is not the contrary function of
* CcRosMapVacbInKernelSpace()
*/
NTSTATUS NTSTATUS
NTAPI NTAPI
CcRosUnmapVacb ( CcRosUnmapVacb (
@ -605,28 +603,22 @@ CcRosUnmapVacb (
return STATUS_UNSUCCESSFUL; return STATUS_UNSUCCESSFUL;
} }
if (NowDirty && !Vacb->Dirty)
{
CcRosMarkDirtyVacb(Vacb);
}
ASSERT(Vacb->MappedCount != 0); ASSERT(Vacb->MappedCount != 0);
Vacb->MappedCount--; Vacb->MappedCount--;
CcRosVacbDecRefCount(Vacb);
if (Vacb->MappedCount == 0) if (Vacb->MappedCount == 0)
{ {
CcRosVacbDecRefCount(Vacb); CcRosVacbDecRefCount(Vacb);
} }
CcRosReleaseVacbLock(Vacb); CcRosReleaseVacb(SharedCacheMap, Vacb, Vacb->Valid, NowDirty, FALSE);
return STATUS_SUCCESS; return STATUS_SUCCESS;
} }
static static
NTSTATUS NTSTATUS
CcRosMapVacb( CcRosMapVacbInKernelSpace(
PROS_VACB Vacb) PROS_VACB Vacb)
{ {
ULONG i; ULONG i;
@ -721,7 +713,7 @@ CcRosCreateVacb (
current->MappedCount = 0; current->MappedCount = 0;
current->DirtyVacbListEntry.Flink = NULL; current->DirtyVacbListEntry.Flink = NULL;
current->DirtyVacbListEntry.Blink = NULL; current->DirtyVacbListEntry.Blink = NULL;
current->ReferenceCount = 1; current->ReferenceCount = 0;
current->PinCount = 0; current->PinCount = 0;
KeInitializeMutex(&current->Mutex, 0); KeInitializeMutex(&current->Mutex, 0);
CcRosAcquireVacbLock(current, NULL); CcRosAcquireVacbLock(current, NULL);
@ -785,6 +777,7 @@ CcRosCreateVacb (
} }
KeReleaseSpinLock(&SharedCacheMap->CacheMapLock, oldIrql); KeReleaseSpinLock(&SharedCacheMap->CacheMapLock, oldIrql);
InsertTailList(&VacbLruListHead, &current->VacbLruListEntry); InsertTailList(&VacbLruListHead, &current->VacbLruListEntry);
CcRosVacbIncRefCount(current);
KeReleaseGuardedMutex(&ViewLock); KeReleaseGuardedMutex(&ViewLock);
MI_SET_USAGE(MI_USAGE_CACHE); MI_SET_USAGE(MI_USAGE_CACHE);
@ -806,7 +799,7 @@ CcRosCreateVacb (
} }
#endif #endif
Status = CcRosMapVacb(current); Status = CcRosMapVacbInKernelSpace(current);
if (!NT_SUCCESS(Status)) if (!NT_SUCCESS(Status))
{ {
RemoveEntryList(&current->CacheMapVacbListEntry); RemoveEntryList(&current->CacheMapVacbListEntry);
@ -849,6 +842,8 @@ CcRosGetVacb (
{ {
return Status; return Status;
} }
CcRosVacbIncRefCount(current);
} }
KeAcquireGuardedMutex(&ViewLock); KeAcquireGuardedMutex(&ViewLock);
@ -941,6 +936,13 @@ CcRosInternalFreeVacb (
NULL); NULL);
MmUnlockAddressSpace(MmGetKernelAddressSpace()); 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); ExFreeToNPagedLookasideList(&VacbLookasideList, Vacb);
return STATUS_SUCCESS; return STATUS_SUCCESS;
} }
@ -1092,6 +1094,7 @@ CcRosDeleteFileCache (
{ {
current_entry = RemoveTailList(&FreeList); current_entry = RemoveTailList(&FreeList);
current = CONTAINING_RECORD(current_entry, ROS_VACB, CacheMapVacbListEntry); current = CONTAINING_RECORD(current_entry, ROS_VACB, CacheMapVacbListEntry);
CcRosVacbDecRefCount(current);
CcRosInternalFreeVacb(current); CcRosInternalFreeVacb(current);
} }

View file

@ -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