mirror of
https://github.com/reactos/reactos.git
synced 2024-12-28 01:55:19 +00:00
[NTOSKRNL] Fix refcounting for BCBs
Now, we make sure that we update ref count and BCB list membership with the BCB lock held, in a row. This will avoid race conditions where the BCB was removed from the list, then referenced again, leading to inconsistencies in memory and crashes later on. This could notably be triggered while building ReactOS on ReactOS (one would call this a regression). CORE-15235
This commit is contained in:
Pierre Schweitzer
parent
0c88f79480
commit
cf7969fbfa
1 changed files with 71 additions and 49 deletions
|
|
@ -152,6 +152,38 @@ CcpMapData(
|
|||
return TRUE;
|
||||
}
|
||||
|
||||
static
|
||||
VOID
|
||||
CcpDereferenceBcb(
|
||||
IN PROS_SHARED_CACHE_MAP SharedCacheMap,
|
||||
IN PINTERNAL_BCB Bcb)
|
||||
{
|
||||
ULONG RefCount;
|
||||
KIRQL OldIrql;
|
||||
|
||||
KeAcquireSpinLock(&SharedCacheMap->BcbSpinLock, &OldIrql);
|
||||
RefCount = --Bcb->RefCount;
|
||||
if (RefCount == 0)
|
||||
{
|
||||
RemoveEntryList(&Bcb->BcbEntry);
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
ASSERT(Bcb->PinCount == 0);
|
||||
CcRosReleaseVacb(SharedCacheMap,
|
||||
Bcb->Vacb,
|
||||
TRUE,
|
||||
Bcb->Dirty,
|
||||
FALSE);
|
||||
|
||||
ExDeleteResourceLite(&Bcb->Lock);
|
||||
ExFreeToNPagedLookasideList(&iBcbLookasideList, Bcb);
|
||||
}
|
||||
else
|
||||
{
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
}
|
||||
}
|
||||
|
||||
static
|
||||
PVOID
|
||||
CcpGetAppropriateBcb(
|
||||
|
|
@ -191,12 +223,13 @@ CcpGetAppropriateBcb(
|
|||
/* Yes, and we've lost */
|
||||
if (DupBcb != NULL)
|
||||
{
|
||||
/* We will return that BCB */
|
||||
++DupBcb->RefCount;
|
||||
Result = TRUE;
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
if (ToPin)
|
||||
{
|
||||
DupBcb->PinCount++;
|
||||
|
||||
if (BooleanFlagOn(PinFlags, PIN_EXCLUSIVE))
|
||||
{
|
||||
Result = ExAcquireResourceExclusiveLite(&iBcb->Lock, BooleanFlagOn(PinFlags, PIN_WAIT));
|
||||
|
|
@ -206,24 +239,25 @@ CcpGetAppropriateBcb(
|
|||
Result = ExAcquireSharedStarveExclusive(&iBcb->Lock, BooleanFlagOn(PinFlags, PIN_WAIT));
|
||||
}
|
||||
|
||||
if (!Result)
|
||||
if (Result)
|
||||
{
|
||||
DupBcb->PinCount--;
|
||||
DupBcb->PinCount++;
|
||||
}
|
||||
else
|
||||
{
|
||||
CcpDereferenceBcb(SharedCacheMap, DupBcb);
|
||||
DupBcb = NULL;
|
||||
}
|
||||
}
|
||||
|
||||
if (Result)
|
||||
if (DupBcb != NULL)
|
||||
{
|
||||
/* We'll return that BCB */
|
||||
++DupBcb->RefCount;
|
||||
/* Delete the loser */
|
||||
CcRosReleaseVacb(SharedCacheMap, Vacb, TRUE, FALSE, FALSE);
|
||||
ExDeleteResourceLite(&iBcb->Lock);
|
||||
ExFreeToNPagedLookasideList(&iBcbLookasideList, iBcb);
|
||||
}
|
||||
|
||||
/* Delete the loser */
|
||||
CcRosReleaseVacb(SharedCacheMap, Vacb, TRUE, FALSE, FALSE);
|
||||
ExDeleteResourceLite(&iBcb->Lock);
|
||||
ExFreeToNPagedLookasideList(&iBcbLookasideList, iBcb);
|
||||
|
||||
/* Return the winner - no need to update buffer address, it's
|
||||
* relative to the VACB, which is unchanged.
|
||||
*/
|
||||
|
|
@ -249,8 +283,8 @@ CcpGetAppropriateBcb(
|
|||
}
|
||||
|
||||
InsertTailList(&SharedCacheMap->BcbList, &iBcb->BcbEntry);
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
}
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
return iBcb;
|
||||
}
|
||||
|
|
@ -273,11 +307,11 @@ CcpPinData(
|
|||
|
||||
KeAcquireSpinLock(&SharedCacheMap->BcbSpinLock, &OldIrql);
|
||||
NewBcb = CcpFindBcb(SharedCacheMap, FileOffset, Length, TRUE);
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
if (NewBcb != NULL)
|
||||
{
|
||||
NewBcb->PinCount++;
|
||||
++NewBcb->RefCount;
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
if (BooleanFlagOn(Flags, PIN_EXCLUSIVE))
|
||||
{
|
||||
|
|
@ -290,11 +324,12 @@ CcpPinData(
|
|||
|
||||
if (!Result)
|
||||
{
|
||||
NewBcb->PinCount--;
|
||||
CcpDereferenceBcb(SharedCacheMap, NewBcb);
|
||||
NewBcb = NULL;
|
||||
}
|
||||
else
|
||||
{
|
||||
NewBcb->RefCount++;
|
||||
NewBcb->PinCount++;
|
||||
*Bcb = NewBcb;
|
||||
*Buffer = (PUCHAR)NewBcb->Vacb->BaseAddress + FileOffset->QuadPart % VACB_MAPPING_GRANULARITY;
|
||||
}
|
||||
|
|
@ -303,6 +338,8 @@ CcpPinData(
|
|||
}
|
||||
else
|
||||
{
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
if (BooleanFlagOn(Flags, PIN_IF_BCB))
|
||||
{
|
||||
return FALSE;
|
||||
|
|
@ -374,10 +411,11 @@ CcMapData (
|
|||
|
||||
KeAcquireSpinLock(&SharedCacheMap->BcbSpinLock, &OldIrql);
|
||||
iBcb = CcpFindBcb(SharedCacheMap, FileOffset, Length, FALSE);
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
if (iBcb == NULL)
|
||||
{
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
Ret = CcpMapData(SharedCacheMap, FileOffset, Length, Flags, &Vacb, pBuffer);
|
||||
if (Ret)
|
||||
{
|
||||
|
|
@ -396,6 +434,8 @@ CcMapData (
|
|||
else
|
||||
{
|
||||
++iBcb->RefCount;
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
*pBcb = iBcb;
|
||||
*pBuffer = (PUCHAR)iBcb->Vacb->BaseAddress + FileOffset->QuadPart % VACB_MAPPING_GRANULARITY;
|
||||
Ret = TRUE;
|
||||
|
|
@ -564,6 +604,7 @@ CcUnpinDataForThread (
|
|||
IN ERESOURCE_THREAD ResourceThreadId)
|
||||
{
|
||||
PINTERNAL_BCB iBcb = Bcb;
|
||||
PROS_SHARED_CACHE_MAP SharedCacheMap;
|
||||
|
||||
CCTRACE(CC_API_DEBUG, "Bcb=%p ResourceThreadId=%lu\n", Bcb, ResourceThreadId);
|
||||
|
||||
|
|
@ -573,29 +614,8 @@ CcUnpinDataForThread (
|
|||
iBcb->PinCount--;
|
||||
}
|
||||
|
||||
if (--iBcb->RefCount == 0)
|
||||
{
|
||||
KIRQL OldIrql;
|
||||
PROS_SHARED_CACHE_MAP SharedCacheMap;
|
||||
|
||||
ASSERT(iBcb->PinCount == 0);
|
||||
SharedCacheMap = iBcb->Vacb->SharedCacheMap;
|
||||
CcRosReleaseVacb(SharedCacheMap,
|
||||
iBcb->Vacb,
|
||||
TRUE,
|
||||
iBcb->Dirty,
|
||||
FALSE);
|
||||
|
||||
KeAcquireSpinLock(&SharedCacheMap->BcbSpinLock, &OldIrql);
|
||||
if (!IsListEmpty(&iBcb->BcbEntry))
|
||||
{
|
||||
RemoveEntryList(&iBcb->BcbEntry);
|
||||
}
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
ExDeleteResourceLite(&iBcb->Lock);
|
||||
ExFreeToNPagedLookasideList(&iBcbLookasideList, iBcb);
|
||||
}
|
||||
SharedCacheMap = iBcb->Vacb->SharedCacheMap;
|
||||
CcpDereferenceBcb(SharedCacheMap, iBcb);
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
@ -629,9 +649,15 @@ CcUnpinRepinnedBcb (
|
|||
|
||||
CCTRACE(CC_API_DEBUG, "Bcb=%p WriteThrough=%d\n", Bcb, WriteThrough);
|
||||
|
||||
SharedCacheMap = iBcb->Vacb->SharedCacheMap;
|
||||
IoStatus->Status = STATUS_SUCCESS;
|
||||
|
||||
KeAcquireSpinLock(&SharedCacheMap->BcbSpinLock, &OldIrql);
|
||||
if (--iBcb->RefCount == 0)
|
||||
{
|
||||
RemoveEntryList(&iBcb->BcbEntry);
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
IoStatus->Information = 0;
|
||||
if (WriteThrough)
|
||||
{
|
||||
|
|
@ -656,21 +682,17 @@ CcUnpinRepinnedBcb (
|
|||
ASSERT(iBcb->PinCount == 0);
|
||||
}
|
||||
|
||||
SharedCacheMap = iBcb->Vacb->SharedCacheMap;
|
||||
CcRosReleaseVacb(iBcb->Vacb->SharedCacheMap,
|
||||
iBcb->Vacb,
|
||||
TRUE,
|
||||
iBcb->Dirty,
|
||||
FALSE);
|
||||
|
||||
KeAcquireSpinLock(&SharedCacheMap->BcbSpinLock, &OldIrql);
|
||||
if (!IsListEmpty(&iBcb->BcbEntry))
|
||||
{
|
||||
RemoveEntryList(&iBcb->BcbEntry);
|
||||
}
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
|
||||
ExDeleteResourceLite(&iBcb->Lock);
|
||||
ExFreeToNPagedLookasideList(&iBcbLookasideList, iBcb);
|
||||
}
|
||||
else
|
||||
{
|
||||
KeReleaseSpinLock(&SharedCacheMap->BcbSpinLock, OldIrql);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue