diff --git a/src/sys/file.c b/src/sys/file.c index a5857670..3151f0af 100644 --- a/src/sys/file.c +++ b/src/sys/file.c @@ -1039,9 +1039,52 @@ VOID FspFileNodeSetFileInfo(FSP_FILE_NODE *FileNode, PFILE_OBJECT CcFileObject, CcFileObject, (PCC_FILE_SIZES)&FileNode->Header.AllocationSize); if (!NT_SUCCESS(Result)) { - DEBUGLOG("FspCcSetFileSizes error: %s", NtStatusSym(Result)); - DEBUGBREAK_CRIT(); - CcUninitializeCacheMap(CcFileObject, 0, 0); + /* + * CcSetFileSizes failed. This is a hard case to handle, because it is + * usually late in IRP processing. So we have the following strategy. + * + * Our goal is to completely stop all caching for this FileNode. The idea + * is that if some I/O arrives later for this FileNode, CcInitializeCacheMap + * will be executed (and possibly fail safely there). In fact we may decide + * later to make such CcInitializeCacheMap failures benign (by not using the + * cache when we cannot). + * + * In order to completely stop caching for the FileNode we do the following: + * + * - We flush the cache using CcFlushCache. + * - We purge the cache and uninitialize all PrivateCacheMap's using + * CcPurgeCacheSection with UninitializeCacheMaps==TRUE. + * - If the SharedCacheMap is still around, we perform an additional + * CcUninitializeCacheMap with an UninitializeEvent. At this point + * CcUninitializeCacheMap should delete the SharedCacheMap and + * signal the UninitializeEvent. + * + * One potential gotcha is whether there is any possibility for another + * system component to delay deletion of the SharedCacheMap and signaling + * of the UninitializeEvent. This could result in a deadlock, because we + * are already holding the FileNode exclusive and waiting for the + * UninitializeEvent. But the thread that would signal our event would have + * to first acquire our FileNode. Classic deadlock. + * + * I believe (but cannot prove) that this deadlock cannot happen. The reason + * is that we have flushed and purged the cache and we have closed all + * PrivateCacheMap's using this SharedCacheMap. There should be no reason for + * any system component to keep the SharedCacheMap around (famous last words). + */ + + IO_STATUS_BLOCK IoStatus; + CACHE_UNINITIALIZE_EVENT UninitializeEvent; + + FspCcFlushCache(CcFileObject->SectionObjectPointer, 0, 0, &IoStatus); + CcPurgeCacheSection(CcFileObject->SectionObjectPointer, 0, 0, TRUE); + if (0 != CcFileObject->SectionObjectPointer->SharedCacheMap) + { + UninitializeEvent.Next = 0; + KeInitializeEvent(&UninitializeEvent.Event, NotificationEvent, FALSE); + BOOLEAN CacheStopped = CcUninitializeCacheMap(CcFileObject, 0, &UninitializeEvent); + ASSERT(CacheStopped); + KeWaitForSingleObject(&UninitializeEvent.Event, Executive, KernelMode, FALSE, 0); + } } } } diff --git a/src/sys/write.c b/src/sys/write.c index 24e5e8df..9f141da3 100644 --- a/src/sys/write.c +++ b/src/sys/write.c @@ -191,6 +191,13 @@ static NTSTATUS FspFsvolWriteCached( FspFileNodeRelease(FileNode, Main); return Result; } + + /* double-check that the cache still exists in case CcSetFileSizes failed */ + if (0 == FileObject->SectionObjectPointer->SharedCacheMap) + { + FspFileNodeRelease(FileNode, Main); + return STATUS_INSUFFICIENT_RESOURCES; // or STATUS_SECTION_TOO_BIG? + } } /* diff --git a/tools/run-tests.bat b/tools/run-tests.bat index 6c04d9b6..2f36e538 100644 --- a/tools/run-tests.bat +++ b/tools/run-tests.bat @@ -261,6 +261,9 @@ for %%m in (^ fscrash-x64 --terminate --mask=0x%%m --leave >nul 2>&1 if !ERRORLEVEL! neq -1073741823 goto fail ) +echo fscrash-x64 --huge-alloc-size --cached +fscrash-x64 --huge-alloc-size --cached >nul 2>&1 +if !ERRORLEVEL! neq 1 goto fail exit /b 0 :fscrash-x86 @@ -277,6 +280,9 @@ for %%m in (^ fscrash-x86 --terminate --mask=0x%%m --leave >nul 2>&1 if !ERRORLEVEL! neq -1073741823 goto fail ) +echo fscrash-x86 --huge-alloc-size --cached +fscrash-x86 --huge-alloc-size --cached >nul 2>&1 +if !ERRORLEVEL! neq 1 goto fail exit /b 0 :leak-test diff --git a/tst/fscrash/fscrash-main.c b/tst/fscrash/fscrash-main.c index 586bdb40..ab6db923 100644 --- a/tst/fscrash/fscrash-main.c +++ b/tst/fscrash/fscrash-main.c @@ -68,7 +68,7 @@ static VOID Test(PWSTR Prefix) wsprintfW(FileName, L"%s\\fscrash\\file0", Prefix); Handle = CreateFileW(FileName, GENERIC_ALL, 0, 0, - CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL | FILE_FLAG_NO_BUFFERING, 0); + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0); ASSERT(INVALID_HANDLE_VALUE != Handle); Success = CloseHandle(Handle); ASSERT(Success); @@ -76,7 +76,7 @@ static VOID Test(PWSTR Prefix) wsprintfW(FileName, L"%s\\fscrash\\file0", Prefix); Handle = CreateFileW(FileName, GENERIC_ALL, 0, 0, - CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL | FILE_FLAG_NO_BUFFERING, 0); + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0); ASSERT(INVALID_HANDLE_VALUE != Handle); Success = WriteFile(Handle, WrBuffer, sizeof WrBuffer, &BytesTransferred, 0); @@ -205,7 +205,7 @@ static NTSTATUS CreateTestProcess(PWSTR GlobalRoot, PWSTR Prefix, PHANDLE PProce } ULONG OptCrashMask = -1, OptCrashFlags = FspCrashInterceptAccessViolation, OptCrashPercent = 10; -ULONG OptMemfsFlags = MemfsDisk; +ULONG OptMemfsFlags = MemfsDisk, OptFileInfoTimeout = 0; ULONG OptIterations = -1; PWSTR OptPrefix = 0; @@ -227,14 +227,19 @@ int wmain(int argc, wchar_t **argv) OptCrashMask = wcstoul(a + sizeof "--mask=" - 1, 0, 0); else if (0 == wcscmp(L"--crash", a)) { - OptCrashFlags &= ~FspCrashInterceptTerminate; + OptCrashFlags &= ~FspCrashInterceptMask; OptCrashFlags |= FspCrashInterceptAccessViolation; } else if (0 == wcscmp(L"--terminate", a)) { - OptCrashFlags &= ~FspCrashInterceptAccessViolation; + OptCrashFlags &= ~FspCrashInterceptMask; OptCrashFlags |= FspCrashInterceptTerminate; } + else if (0 == wcscmp(L"--huge-alloc-size", a)) + { + OptCrashFlags &= ~FspCrashInterceptMask; + OptCrashFlags |= FspCrashInterceptHugeAllocationSize; + } else if (0 == wcscmp(L"--enter", a)) OptCrashFlags |= FspCrashInterceptEnter; else if (0 == wcscmp(L"--leave", a)) @@ -245,10 +250,19 @@ int wmain(int argc, wchar_t **argv) OptMemfsFlags = MemfsDisk; else if (0 == wcscmp(L"--net", a)) OptMemfsFlags = MemfsNet; + else if (0 == wcscmp(L"--non-cached", a)) + OptFileInfoTimeout = 0; + else if (0 == wcscmp(L"--cached", a)) + OptFileInfoTimeout = -1; else if (0 == wcsncmp(L"--iterations=", a, sizeof "--iterations=" - 1)) OptIterations = wcstoul(a + sizeof "--iterations=" - 1, 0, 10); else if (0 == wcsncmp(L"--run-test=", a, sizeof "--run-test=" - 1)) OptPrefix = a + sizeof "--run-test=" - 1; + else + { + fail("unknown option %S", a); + exit(2); + } } } @@ -261,7 +275,7 @@ int wmain(int argc, wchar_t **argv) Result = MemfsCreate( OptMemfsFlags, - 0, + OptFileInfoTimeout, 1024, 1024 * 1024, (MemfsNet & OptMemfsFlags) ? L"\\memfs\\share" : 0, diff --git a/tst/fscrash/fscrash.c b/tst/fscrash/fscrash.c index 57990bf4..6e256f65 100644 --- a/tst/fscrash/fscrash.c +++ b/tst/fscrash/fscrash.c @@ -19,13 +19,14 @@ static SRWLOCK FspCrashLock = SRWLOCK_INIT; static FSP_FILE_SYSTEM *FspCrashFileSystem; +static BOOLEAN FspCrashInterceptFlag; static ULONG FspCrashPercent; static FSP_FILE_SYSTEM_OPERATION *FspCrashInterceptedOperations [ARRAYSIZE(((FSP_FILE_SYSTEM *)0)->Operations)]; static volatile PULONG FspCrashNullPointer; static unsigned FspCrashRandSeed = 1; -static int FspCrashRand(void) +static __forceinline int FspCrashRand(void) { /* * This mimics MSVCRT rand(); we need our own version @@ -40,55 +41,93 @@ static __forceinline BOOLEAN FspCrashInterceptTest(FSP_FILE_SYSTEM *FileSystem) { BOOLEAN Result; AcquireSRWLockShared(&FspCrashLock); - Result = FileSystem == FspCrashFileSystem && - FspCrashRand() < (LONG)FspCrashPercent * 0x7fff / 100; + Result = FspCrashInterceptFlag || + (FileSystem == FspCrashFileSystem && + FspCrashRand() < (LONG)FspCrashPercent * 0x7fff / 100); + FspCrashInterceptFlag = FspCrashInterceptFlag || Result; ReleaseSRWLockShared(&FspCrashLock); return Result; } -#define DefineInterceptor(NAME, CRASH, ENTER, LEAVE)\ +#define DefineInterceptor(NAME, ACTION, ENTER, LEAVE)\ static NTSTATUS NAME(FSP_FILE_SYSTEM *FileSystem,\ FSP_FSCTL_TRANSACT_REQ *Request, FSP_FSCTL_TRANSACT_RSP *Response)\ {\ NTSTATUS Result;\ if (ENTER)\ {\ - if (FspCrashInterceptTest(FileSystem))\ + switch (ACTION)\ {\ - if (CRASH)\ + default:\ + case FspCrashInterceptAccessViolation:\ + if (FspCrashInterceptTest(FileSystem))\ *FspCrashNullPointer = 0x42424242;\ - else\ + break;\ + case FspCrashInterceptTerminate:\ + if (FspCrashInterceptTest(FileSystem))\ TerminateProcess(GetCurrentProcess(), STATUS_UNSUCCESSFUL);\ + break;\ + case FspCrashInterceptHugeAllocationSize:\ + break;\ }\ }\ Result = FspCrashInterceptedOperations[Request->Kind](FileSystem, Request, Response);\ if (LEAVE)\ {\ - if (FspCrashInterceptTest(FileSystem))\ + switch (ACTION)\ {\ - if (CRASH)\ + default:\ + case FspCrashInterceptAccessViolation:\ + if (FspCrashInterceptTest(FileSystem))\ *FspCrashNullPointer = 0x42424242;\ - else\ + break;\ + case FspCrashInterceptTerminate:\ + if (FspCrashInterceptTest(FileSystem))\ TerminateProcess(GetCurrentProcess(), STATUS_UNSUCCESSFUL);\ + break;\ + case FspCrashInterceptHugeAllocationSize:\ + if (FspCrashInterceptTest(FileSystem))\ + if (STATUS_SUCCESS == Response->IoStatus.Status)\ + {\ + if (FspFsctlTransactCreateKind == Request->Kind)\ + Response->Rsp.Create.Opened.FileInfo.AllocationSize = HugeAllocationSize;\ + else if (FspFsctlTransactOverwriteKind == Request->Kind)\ + Response->Rsp.Overwrite.FileInfo.AllocationSize = HugeAllocationSize;\ + else if (FspFsctlTransactQueryInformationKind == Request->Kind)\ + Response->Rsp.QueryInformation.FileInfo.AllocationSize = HugeAllocationSize;\ + else if (FspFsctlTransactSetInformationKind == Request->Kind &&\ + (4/*Basic*/ == Request->Req.SetInformation.FileInformationClass ||\ + 19/*Allocation*/ == Request->Req.SetInformation.FileInformationClass ||\ + 20/*EndOfFile*/ == Request->Req.SetInformation.FileInformationClass))\ + Response->Rsp.SetInformation.FileInfo.AllocationSize = HugeAllocationSize;\ + else if (FspFsctlTransactWriteKind == Request->Kind &&\ + !Request->Req.Write.ConstrainedIo)\ + Response->Rsp.Write.FileInfo.AllocationSize = HugeAllocationSize;\ + }\ + break;\ }\ }\ return Result;\ } -DefineInterceptor(FspCrashInterceptorCE, TRUE, TRUE, FALSE) -DefineInterceptor(FspCrashInterceptorCL, TRUE, FALSE, TRUE) -DefineInterceptor(FspCrashInterceptorCB, TRUE, TRUE, TRUE) -DefineInterceptor(FspCrashInterceptorTE, FALSE, TRUE, FALSE) -DefineInterceptor(FspCrashInterceptorTL, FALSE, FALSE, TRUE) -DefineInterceptor(FspCrashInterceptorTB, FALSE, TRUE, TRUE) +#define HugeAllocationSize 0x1000000000000000ULL +DefineInterceptor(FspCrashInterceptorCE, FspCrashInterceptAccessViolation, TRUE, FALSE) +DefineInterceptor(FspCrashInterceptorCL, FspCrashInterceptAccessViolation, FALSE, TRUE) +DefineInterceptor(FspCrashInterceptorCB, FspCrashInterceptAccessViolation, TRUE, TRUE) +DefineInterceptor(FspCrashInterceptorTE, FspCrashInterceptTerminate, TRUE, FALSE) +DefineInterceptor(FspCrashInterceptorTL, FspCrashInterceptTerminate, FALSE, TRUE) +DefineInterceptor(FspCrashInterceptorTB, FspCrashInterceptTerminate, TRUE, TRUE) +DefineInterceptor(FspCrashInterceptorAB, FspCrashInterceptHugeAllocationSize, TRUE, TRUE) VOID FspCrashIntercept(FSP_FILE_SYSTEM *FileSystem, ULONG CrashMask, ULONG CrashFlags, ULONG CrashPercent) { FSP_FILE_SYSTEM_OPERATION *Interceptor = 0; - if (CrashFlags & FspCrashInterceptAccessViolation) + switch (CrashFlags & FspCrashInterceptMask) { + default: + case FspCrashInterceptAccessViolation: if (FspCrashInterceptEnter == (CrashFlags & FspCrashInterceptEnter)) Interceptor = FspCrashInterceptorCE; else @@ -96,9 +135,8 @@ VOID FspCrashIntercept(FSP_FILE_SYSTEM *FileSystem, Interceptor = FspCrashInterceptorCL; else Interceptor = FspCrashInterceptorCB; - } - else - { + break; + case FspCrashInterceptTerminate: if (FspCrashInterceptEnter == (CrashFlags & FspCrashInterceptEnter)) Interceptor = FspCrashInterceptorTE; else @@ -106,6 +144,10 @@ VOID FspCrashIntercept(FSP_FILE_SYSTEM *FileSystem, Interceptor = FspCrashInterceptorTL; else Interceptor = FspCrashInterceptorTB; + break; + case FspCrashInterceptHugeAllocationSize: + Interceptor = FspCrashInterceptorAB; + break; } RtlCopyMemory(FspCrashInterceptedOperations, diff --git a/tst/fscrash/fscrash.h b/tst/fscrash/fscrash.h index feeec606..002c6f26 100644 --- a/tst/fscrash/fscrash.h +++ b/tst/fscrash/fscrash.h @@ -26,8 +26,10 @@ extern "C" { enum { - FspCrashInterceptAccessViolation = 0x01, - FspCrashInterceptTerminate = 0x02, + FspCrashInterceptMask = 0x0f, + FspCrashInterceptAccessViolation = 0x00, + FspCrashInterceptTerminate = 0x01, + FspCrashInterceptHugeAllocationSize = 0x02, FspCrashInterceptEnter = 0x10, FspCrashInterceptLeave = 0x20, };