[NTOSKRNL] Move the PinCount out of the VACB to the BCB

Given current ReactOS implementation, a VACB can be pinned
several times, with different BCB. In next commits, a single
BCB will be able to be pinned several times. That would
lead to severe inconsistencies in counting and thus corruption.
This commit is contained in:
Pierre Schweitzer 2018-10-05 19:43:10 +02:00
parent f9739601b3
commit f284947622
No known key found for this signature in database
GPG key ID: 7545556C3D585B0B
3 changed files with 14 additions and 21 deletions

View file

@ -136,7 +136,7 @@ CcpMapData(
iBcb->PFCB.MappedFileOffset = *FileOffset; iBcb->PFCB.MappedFileOffset = *FileOffset;
iBcb->Vacb = Vacb; iBcb->Vacb = Vacb;
iBcb->Dirty = FALSE; iBcb->Dirty = FALSE;
iBcb->Pinned = FALSE; iBcb->PinCount = 0;
iBcb->RefCount = 1; iBcb->RefCount = 1;
ExInitializeResourceLite(&iBcb->Lock); ExInitializeResourceLite(&iBcb->Lock);
*pBcb = (PVOID)iBcb; *pBcb = (PVOID)iBcb;
@ -215,10 +215,9 @@ CcPinMappedData (
} }
iBcb = *Bcb; iBcb = *Bcb;
ASSERT(iBcb->Pinned == FALSE); ASSERT(iBcb->PinCount == 0);
iBcb->Pinned = TRUE; iBcb->PinCount++;
iBcb->Vacb->PinCount++;
if (BooleanFlagOn(Flags, PIN_EXCLUSIVE)) if (BooleanFlagOn(Flags, PIN_EXCLUSIVE))
{ {
@ -231,8 +230,7 @@ CcPinMappedData (
if (!Result) if (!Result)
{ {
iBcb->Pinned = FALSE; iBcb->PinCount--;
iBcb->Vacb->PinCount--;
} }
return Result; return Result;
@ -353,11 +351,10 @@ CcUnpinDataForThread (
CCTRACE(CC_API_DEBUG, "Bcb=%p ResourceThreadId=%lu\n", Bcb, ResourceThreadId); CCTRACE(CC_API_DEBUG, "Bcb=%p ResourceThreadId=%lu\n", Bcb, ResourceThreadId);
if (iBcb->Pinned) if (iBcb->PinCount != 0)
{ {
ExReleaseResourceForThreadLite(&iBcb->Lock, ResourceThreadId); ExReleaseResourceForThreadLite(&iBcb->Lock, ResourceThreadId);
iBcb->Pinned = FALSE; iBcb->PinCount--;
iBcb->Vacb->PinCount--;
} }
if (--iBcb->RefCount == 0) if (--iBcb->RefCount == 0)
@ -365,6 +362,7 @@ CcUnpinDataForThread (
KIRQL OldIrql; KIRQL OldIrql;
PROS_SHARED_CACHE_MAP SharedCacheMap; PROS_SHARED_CACHE_MAP SharedCacheMap;
ASSERT(iBcb->PinCount == 0);
SharedCacheMap = iBcb->Vacb->SharedCacheMap; SharedCacheMap = iBcb->Vacb->SharedCacheMap;
CcRosReleaseVacb(SharedCacheMap, CcRosReleaseVacb(SharedCacheMap,
iBcb->Vacb, iBcb->Vacb,
@ -432,12 +430,11 @@ CcUnpinRepinnedBcb (
IoStatus->Status = STATUS_SUCCESS; IoStatus->Status = STATUS_SUCCESS;
} }
if (iBcb->Pinned) if (iBcb->PinCount != 0)
{ {
ExReleaseResourceLite(&iBcb->Lock); ExReleaseResourceLite(&iBcb->Lock);
iBcb->Pinned = FALSE; iBcb->PinCount--;
iBcb->Vacb->PinCount--; ASSERT(iBcb->PinCount == 0);
ASSERT(iBcb->Vacb->PinCount == 0);
} }
SharedCacheMap = iBcb->Vacb->SharedCacheMap; SharedCacheMap = iBcb->Vacb->SharedCacheMap;

View file

@ -808,7 +808,6 @@ CcRosCreateVacb (
#endif #endif
current->MappedCount = 0; current->MappedCount = 0;
current->ReferenceCount = 0; current->ReferenceCount = 0;
current->PinCount = 0;
InitializeListHead(&current->CacheMapVacbListEntry); InitializeListHead(&current->CacheMapVacbListEntry);
InitializeListHead(&current->DirtyVacbListEntry); InitializeListHead(&current->DirtyVacbListEntry);
InitializeListHead(&current->VacbLruListEntry); InitializeListHead(&current->VacbLruListEntry);
@ -1056,16 +1055,15 @@ CcRosInternalFreeVacb (
NULL); NULL);
MmUnlockAddressSpace(MmGetKernelAddressSpace()); MmUnlockAddressSpace(MmGetKernelAddressSpace());
if (Vacb->PinCount != 0 || Vacb->ReferenceCount != 0) if (Vacb->ReferenceCount != 0)
{ {
DPRINT1("Invalid free: %ld, %ld\n", Vacb->ReferenceCount, Vacb->PinCount); DPRINT1("Invalid free: %ld\n", Vacb->ReferenceCount);
if (Vacb->SharedCacheMap->FileObject && Vacb->SharedCacheMap->FileObject->FileName.Length) if (Vacb->SharedCacheMap->FileObject && Vacb->SharedCacheMap->FileObject->FileName.Length)
{ {
DPRINT1("For file: %wZ\n", &Vacb->SharedCacheMap->FileObject->FileName); DPRINT1("For file: %wZ\n", &Vacb->SharedCacheMap->FileObject->FileName);
} }
} }
ASSERT(Vacb->PinCount == 0);
ASSERT(Vacb->ReferenceCount == 0); ASSERT(Vacb->ReferenceCount == 0);
ASSERT(IsListEmpty(&Vacb->CacheMapVacbListEntry)); ASSERT(IsListEmpty(&Vacb->CacheMapVacbListEntry));
ASSERT(IsListEmpty(&Vacb->DirtyVacbListEntry)); ASSERT(IsListEmpty(&Vacb->DirtyVacbListEntry));
@ -1229,7 +1227,7 @@ CcRosDeleteFileCache (
{ {
DPRINT1("Leaking VACB %p attached to %p (%I64d)\n", current, FileObject, current->FileOffset.QuadPart); DPRINT1("Leaking VACB %p attached to %p (%I64d)\n", current, FileObject, current->FileOffset.QuadPart);
DPRINT1("There are: %d references left\n", Refs); DPRINT1("There are: %d references left\n", Refs);
DPRINT1("Pin: %d, Map: %d\n", current->PinCount, current->MappedCount); DPRINT1("Map: %d\n", current->MappedCount);
DPRINT1("Dirty: %d\n", current->Dirty); DPRINT1("Dirty: %d\n", current->Dirty);
if (FileObject->FileName.Length != 0) if (FileObject->FileName.Length != 0)
{ {

View file

@ -221,8 +221,6 @@ typedef struct _ROS_VACB
LARGE_INTEGER FileOffset; LARGE_INTEGER FileOffset;
/* Number of references. */ /* Number of references. */
volatile ULONG ReferenceCount; volatile ULONG ReferenceCount;
/* How many times was it pinned? */
LONG PinCount;
/* Pointer to the shared cache map for the file which this view maps data for. */ /* Pointer to the shared cache map for the file which this view maps data for. */
PROS_SHARED_CACHE_MAP SharedCacheMap; PROS_SHARED_CACHE_MAP SharedCacheMap;
/* Pointer to the next VACB in a chain. */ /* Pointer to the next VACB in a chain. */
@ -235,7 +233,7 @@ typedef struct _INTERNAL_BCB
PUBLIC_BCB PFCB; PUBLIC_BCB PFCB;
PROS_VACB Vacb; PROS_VACB Vacb;
BOOLEAN Dirty; BOOLEAN Dirty;
BOOLEAN Pinned; ULONG PinCount;
CSHORT RefCount; /* (At offset 0x34 on WinNT4) */ CSHORT RefCount; /* (At offset 0x34 on WinNT4) */
LIST_ENTRY BcbEntry; LIST_ENTRY BcbEntry;
} INTERNAL_BCB, *PINTERNAL_BCB; } INTERNAL_BCB, *PINTERNAL_BCB;