Root cause: CcRosDeleteFileCache's first loop ran under the spinlock and removed VACBs from the LRU and dirty lists, but left them in CacheMapVacbListHead. It also set Vacb->Dirty = TRUE as a flush hint (after CcRosUnmarkDirtyVacb cleared it), creating an inconsistent VACB state: Dirty=TRUE but not in the dirty list, with only the cache-map-list refcount (1). After releasing the lock, the second loop removed VACBs from CacheMapVacbListHead without holding any lock.
This created a race with CcFlushCache: a caller that already held a SharedCacheMap pointer could call CcRosLookupVacb between the two loops, find the VACB (refcount → 2), see Dirty=TRUE (the hack), and call CcRosFlushVacb. Meanwhile the second loop could set Dirty=FALSE, drop the cmap ref (→1, print "Leaking VACB"), and then the early-return in CcRosUnmarkDirtyVacb would skip the decrement. CcRosReleaseVacb then drops 1→0 → ASSERT(Refs > 0) fires.
Before the previous fix, the same race caused ASSERT(Vacb->Dirty) in the old CcRosUnmarkDirtyVacb — the previous fix just changed which assert fired.
The fix:
- CcRosDeleteFileCache: VACBs are now removed from CacheMapVacbListHead in the first loop, under the spinlock, and moved to a private LocalVacbList. After the lock is released, CcRosLookupVacb can no longer find these VACBs, preventing new lookup references from being created.
- CcRosReleaseVacb: Removed the overly strict ASSERT(Refs > 0). When CcRosDeleteFileCache drops the cmap ref while a lookup is outstanding, CcRosReleaseVacb legitimately releases the last reference and CcRosVacbDecRefCount correctly frees the VACB. Callers (e.g., CcFlushCache) don't access the VACB pointer afterward.
Signed-off-by: Timo Kreuzer <[email protected]>
- CcRosUnmarkDirtyVacb: return BOOLEAN, guard against double-unmark
- CcRosMarkDirtyVacb: guard against double-insertion
- CcRosFlushVacb: only re-mark dirty on failure if we were the one who unmarked
- Add comments to lockless Dirty reads in CcFlushCache and CcRosReleaseVacb
Signed-off-by: Timo Kreuzer <[email protected]>
Use section object pointer with byte offset instead of using base address. This simplifies the Mm functions themselves and also the code in Cc that calls them.
Also add minor fixes for MmFlushSegment and MmPurgeSegment.
* [NTOS:CC] Restore unlock and reacquire locks around MmPageOutPhysicalAddress which was mistakenly lost in commit 2b14056
* Add bypass for problematic code path for now
Finally handlers are - unlike except blocks - not part of the function they are in, but separate functions, which are called during unwind. PSEH implements them on GCC using nested functions. While "return" from a finally handler is allowed with native SEH, it's handled by the compiler through an extra unwinding operation using _local_unwind, WHICH IS NOT SUPPORTED BY PSEH! With PSEH, returning from a finally handler does not return from the function, instead it will only return from the finally handler and the function will continue below the finally handler as if there was no return at all. To fix this, the return is removed and an additional success check is added.
Also use _SEH_VOLATILE to make sure the variable assignment is not optimized away by the compiler and add zero out the result parameters on error.
Before purging the data cache of a certain section of a file from system cache, we have to unintialize the private cache maps of that section if a filesystem or any other component prompts the kernel to do so.
copilot-swe-agent[bot]