diff --git a/build/VStudio/testing/winfsp-tests.vcxproj b/build/VStudio/testing/winfsp-tests.vcxproj index 0352416a..91878e5e 100644 --- a/build/VStudio/testing/winfsp-tests.vcxproj +++ b/build/VStudio/testing/winfsp-tests.vcxproj @@ -195,6 +195,7 @@ + diff --git a/build/VStudio/testing/winfsp-tests.vcxproj.filters b/build/VStudio/testing/winfsp-tests.vcxproj.filters index b3fcf912..5049319e 100644 --- a/build/VStudio/testing/winfsp-tests.vcxproj.filters +++ b/build/VStudio/testing/winfsp-tests.vcxproj.filters @@ -109,6 +109,9 @@ Source + + Source + diff --git a/src/dll/ntstatus.i b/src/dll/ntstatus.i index fc683d92..15939599 100644 --- a/src/dll/ntstatus.i +++ b/src/dll/ntstatus.i @@ -46,6 +46,7 @@ case ERROR_CANT_ACCESS_FILE: return STATUS_IO_REPARSE_TAG_NOT_H case ERROR_CANT_DISABLE_MANDATORY: return STATUS_CANT_DISABLE_MANDATORY; case ERROR_CANT_OPEN_ANONYMOUS: return STATUS_CANT_OPEN_ANONYMOUS; case ERROR_CANT_RESOLVE_FILENAME: return STATUS_REPARSE_POINT_NOT_RESOLVED; +case ERROR_CANT_WAIT: return STATUS_CANT_WAIT; case ERROR_CHILD_MUST_BE_VOLATILE: return STATUS_CHILD_MUST_BE_VOLATILE; case ERROR_CLEANER_CARTRIDGE_INSTALLED: return STATUS_CLEANER_CARTRIDGE_INSTALLED; case ERROR_CLUSTER_INVALID_NETWORK: return STATUS_CLUSTER_INVALID_NETWORK; diff --git a/src/sys/device.c b/src/sys/device.c index 6fccf8bb..16e1c804 100644 --- a/src/sys/device.c +++ b/src/sys/device.c @@ -41,7 +41,9 @@ static VOID FspFsvolDeviceFini(PDEVICE_OBJECT DeviceObject); static IO_TIMER_ROUTINE FspFsvolDeviceTimerRoutine; static WORKER_THREAD_ROUTINE FspFsvolDeviceExpirationRoutine; VOID FspFsvolDeviceFileRenameAcquireShared(PDEVICE_OBJECT DeviceObject); +BOOLEAN FspFsvolDeviceFileRenameTryAcquireShared(PDEVICE_OBJECT DeviceObject); VOID FspFsvolDeviceFileRenameAcquireExclusive(PDEVICE_OBJECT DeviceObject); +BOOLEAN FspFsvolDeviceFileRenameTryAcquireExclusive(PDEVICE_OBJECT DeviceObject); VOID FspFsvolDeviceFileRenameSetOwner(PDEVICE_OBJECT DeviceObject, PVOID Owner); VOID FspFsvolDeviceFileRenameRelease(PDEVICE_OBJECT DeviceObject); VOID FspFsvolDeviceFileRenameReleaseOwner(PDEVICE_OBJECT DeviceObject, PVOID Owner); @@ -83,7 +85,9 @@ VOID FspDeviceDeleteAll(VOID); #pragma alloc_text(PAGE, FspFsvolDeviceInit) #pragma alloc_text(PAGE, FspFsvolDeviceFini) #pragma alloc_text(PAGE, FspFsvolDeviceFileRenameAcquireShared) +#pragma alloc_text(PAGE, FspFsvolDeviceFileRenameTryAcquireShared) #pragma alloc_text(PAGE, FspFsvolDeviceFileRenameAcquireExclusive) +#pragma alloc_text(PAGE, FspFsvolDeviceFileRenameTryAcquireExclusive) #pragma alloc_text(PAGE, FspFsvolDeviceFileRenameSetOwner) #pragma alloc_text(PAGE, FspFsvolDeviceFileRenameRelease) #pragma alloc_text(PAGE, FspFsvolDeviceFileRenameReleaseOwner) @@ -596,6 +600,15 @@ VOID FspFsvolDeviceFileRenameAcquireShared(PDEVICE_OBJECT DeviceObject) ExAcquireResourceSharedLite(&FsvolDeviceExtension->FileRenameResource, TRUE); } +BOOLEAN FspFsvolDeviceFileRenameTryAcquireShared(PDEVICE_OBJECT DeviceObject) +{ + PAGED_CODE(); + + FSP_FSVOL_DEVICE_EXTENSION *FsvolDeviceExtension = FspFsvolDeviceExtension(DeviceObject); + + return ExAcquireResourceSharedLite(&FsvolDeviceExtension->FileRenameResource, FALSE); +} + VOID FspFsvolDeviceFileRenameAcquireExclusive(PDEVICE_OBJECT DeviceObject) { PAGED_CODE(); @@ -605,6 +618,15 @@ VOID FspFsvolDeviceFileRenameAcquireExclusive(PDEVICE_OBJECT DeviceObject) ExAcquireResourceExclusiveLite(&FsvolDeviceExtension->FileRenameResource, TRUE); } +BOOLEAN FspFsvolDeviceFileRenameTryAcquireExclusive(PDEVICE_OBJECT DeviceObject) +{ + PAGED_CODE(); + + FSP_FSVOL_DEVICE_EXTENSION *FsvolDeviceExtension = FspFsvolDeviceExtension(DeviceObject); + + return ExAcquireResourceExclusiveLite(&FsvolDeviceExtension->FileRenameResource, FALSE); +} + VOID FspFsvolDeviceFileRenameSetOwner(PDEVICE_OBJECT DeviceObject, PVOID Owner) { PAGED_CODE(); diff --git a/src/sys/driver.h b/src/sys/driver.h index 8abbe2ce..5b8acb0d 100644 --- a/src/sys/driver.h +++ b/src/sys/driver.h @@ -1124,6 +1124,7 @@ typedef struct KSPIN_LOCK InfoSpinLock; UINT64 InfoExpirationTime; FSP_FSCTL_VOLUME_INFO VolumeInfo; + LONG VolumeNotifyLock; PNOTIFY_SYNC NotifySync; LIST_ENTRY NotifyList; FSP_STATISTICS *Statistics; @@ -1182,7 +1183,9 @@ VOID FspDeviceDelete(PDEVICE_OBJECT DeviceObject); BOOLEAN FspDeviceReference(PDEVICE_OBJECT DeviceObject); VOID FspDeviceDereference(PDEVICE_OBJECT DeviceObject); VOID FspFsvolDeviceFileRenameAcquireShared(PDEVICE_OBJECT DeviceObject); +BOOLEAN FspFsvolDeviceFileRenameTryAcquireShared(PDEVICE_OBJECT DeviceObject); VOID FspFsvolDeviceFileRenameAcquireExclusive(PDEVICE_OBJECT DeviceObject); +BOOLEAN FspFsvolDeviceFileRenameTryAcquireExclusive(PDEVICE_OBJECT DeviceObject); VOID FspFsvolDeviceFileRenameSetOwner(PDEVICE_OBJECT DeviceObject, PVOID Owner); VOID FspFsvolDeviceFileRenameRelease(PDEVICE_OBJECT DeviceObject); VOID FspFsvolDeviceFileRenameReleaseOwner(PDEVICE_OBJECT DeviceObject, PVOID Owner); diff --git a/src/sys/volume.c b/src/sys/volume.c index 8d35b7b0..f8889235 100644 --- a/src/sys/volume.c +++ b/src/sys/volume.c @@ -52,6 +52,8 @@ NTSTATUS FspVolumeStop( PDEVICE_OBJECT FsctlDeviceObject, PIRP Irp, PIO_STACK_LOCATION IrpSp); NTSTATUS FspVolumeNotify( PDEVICE_OBJECT FsctlDeviceObject, PIRP Irp, PIO_STACK_LOCATION IrpSp); +static NTSTATUS FspVolumeNotifyLock( + PDEVICE_OBJECT FsvolDeviceObject); static WORKER_THREAD_ROUTINE FspVolumeNotifyWork; NTSTATUS FspVolumeWork( PDEVICE_OBJECT FsvolDeviceObject, PIRP Irp, PIO_STACK_LOCATION IrpSp); @@ -72,6 +74,7 @@ NTSTATUS FspVolumeWork( #pragma alloc_text(PAGE, FspVolumeTransactFsext) #pragma alloc_text(PAGE, FspVolumeStop) #pragma alloc_text(PAGE, FspVolumeNotify) +#pragma alloc_text(PAGE, FspVolumeNotifyLock) #pragma alloc_text(PAGE, FspVolumeNotifyWork) #pragma alloc_text(PAGE, FspVolumeWork) #endif @@ -473,6 +476,10 @@ static VOID FspVolumeDeleteNoLock( FspMupUnregister(Globals->FsmupDeviceObject, FsvolDeviceObject); } + /* release the volume notify lock if held (so that any pending rename will abort) */ + if (1 == InterlockedCompareExchange(&FsvolDeviceExtension->VolumeNotifyLock, 0, 1)) + FspFsvolDeviceFileRenameReleaseOwner(FsvolDeviceObject, FsvolDeviceObject); + /* release the volume device object */ FspDeviceDereference(FsvolDeviceObject); } @@ -1095,6 +1102,9 @@ NTSTATUS FspVolumeNotify( FSP_VOLUME_NOTIFY_WORK_ITEM *NotifyWorkItem = 0; NTSTATUS Result; + if (0 == InputBufferLength) + return FspVolumeNotifyLock(FsvolDeviceObject); + if (!FspDeviceReference(FsvolDeviceObject)) return STATUS_CANCELLED; @@ -1145,6 +1155,43 @@ fail: return Result; } +static NTSTATUS FspVolumeNotifyLock( + PDEVICE_OBJECT FsvolDeviceObject) +{ + PAGED_CODE(); + + NTSTATUS Result; + + if (!FspDeviceReference(FsvolDeviceObject)) + return STATUS_CANCELLED; + + /* + * Acquire the rename lock shared to disallow concurrent RENAME's. + * + * This guards against the race where a file that we want to invalidate + * is being concurrently renamed to a different name. Thus we may think + * that the file is not open and not invalidate its caches, whereas the + * file has simply changed name. + */ + Result = STATUS_CANT_WAIT; + if (FspFsvolDeviceFileRenameTryAcquireShared(FsvolDeviceObject)) + { + FSP_FSVOL_DEVICE_EXTENSION *FsvolDeviceExtension = FspFsvolDeviceExtension(FsvolDeviceObject); + + if (0 == InterlockedCompareExchange(&FsvolDeviceExtension->VolumeNotifyLock, 1, 0)) + { + FspFsvolDeviceFileRenameSetOwner(FsvolDeviceObject, FsvolDeviceObject); + Result = STATUS_SUCCESS; + } + else + FspFsvolDeviceFileRenameRelease(FsvolDeviceObject); + } + + FspDeviceDereference(FsvolDeviceObject); + + return Result; +} + static VOID FspVolumeNotifyWork(PVOID NotifyWorkItem0) { PAGED_CODE(); @@ -1162,16 +1209,6 @@ static VOID FspVolumeNotifyWork(PVOID NotifyWorkItem0) ULONG StreamType = FspFileNameStreamTypeNone; NTSTATUS Result; - /* - * Acquire the rename lock shared to disallow concurrent RENAME's. - * - * This guards against the race where a file that we want to invalidate - * is being concurrently renamed to a different name. Thus we may think - * that the file is not open and not invalidate its caches, whereas the - * file has simply changed name. - */ - FspFsvolDeviceFileRenameAcquireShared(FsvolDeviceObject); - /* iterate over notify information and invalidate/notify each file */ for (; (PUINT8)NotifyInfo + sizeof(NotifyInfo->Size) <= NotifyInfoEnd; NotifyInfo = (PVOID)((PUINT8)NotifyInfo + FSP_FSCTL_DEFAULT_ALIGN_UP(NotifyInfoSize))) @@ -1179,7 +1216,11 @@ static VOID FspVolumeNotifyWork(PVOID NotifyWorkItem0) NotifyInfoSize = NotifyInfo->Size; if (sizeof(FSP_FSCTL_NOTIFY_INFO) > NotifyInfoSize) + { + if (1 == InterlockedCompareExchange(&FsvolDeviceExtension->VolumeNotifyLock, 0, 1)) + FspFsvolDeviceFileRenameReleaseOwner(FsvolDeviceObject, FsvolDeviceObject); break; + } FileName.Length = FileName.MaximumLength = (USHORT)(NotifyInfoSize - sizeof(FSP_FSCTL_NOTIFY_INFO)); @@ -1229,8 +1270,6 @@ static VOID FspVolumeNotifyWork(PVOID NotifyWorkItem0) } } - FspFsvolDeviceFileRenameRelease(FsvolDeviceObject); - if (0 != FullFileName.Buffer) FspFree(FullFileName.Buffer); diff --git a/tools/gensrc/ntstatus.txt b/tools/gensrc/ntstatus.txt index e8bad320..1b93a7d7 100644 --- a/tools/gensrc/ntstatus.txt +++ b/tools/gensrc/ntstatus.txt @@ -347,6 +347,7 @@ STATUS_BAD_VALIDATION_CLASS ERROR_BAD_VALIDATION_CLASS STATUS_BAD_TOKEN_TYPE ERROR_BAD_TOKEN_TYPE STATUS_BAD_MASTER_BOOT_RECORD ERROR_INVALID_PARAMETER STATUS_NO_SECURITY_ON_OBJECT ERROR_NO_SECURITY_ON_OBJECT +STATUS_CANT_WAIT ERROR_CANT_WAIT STATUS_CANT_ACCESS_DOMAIN_INFO ERROR_CANT_ACCESS_DOMAIN_INFO STATUS_INVALID_SERVER_STATE ERROR_INVALID_SERVER_STATE STATUS_INVALID_DOMAIN_STATE ERROR_INVALID_DOMAIN_STATE diff --git a/tst/winfsp-tests/notify-test.c b/tst/winfsp-tests/notify-test.c new file mode 100644 index 00000000..3482f731 --- /dev/null +++ b/tst/winfsp-tests/notify-test.c @@ -0,0 +1,121 @@ +/** + * @file notify-test.c + * + * @copyright 2015-2020 Bill Zissimopoulos + */ +/* + * This file is part of WinFsp. + * + * You can redistribute it and/or modify it under the terms of the GNU + * General Public License version 3 as published by the Free Software + * Foundation. + * + * Licensees holding a valid commercial license may use this software + * in accordance with the commercial license agreement provided in + * conjunction with the software. The terms and conditions of any such + * commercial license agreement shall govern, supersede, and render + * ineffective any application of the GPLv3 license to this software, + * notwithstanding of any reference thereto in the software or + * associated repository. + */ + +#include +#include +#include +#include +#include "memfs.h" + +#include "winfsp-tests.h" + +static +void notify_abandon_dotest(ULONG Flags) +{ + void *memfs = memfs_start(Flags); + FSP_FILE_SYSTEM *FileSystem = MemfsFileSystem(memfs); + NTSTATUS Result; + + Result = FspFsctlNotify(FileSystem->VolumeHandle, 0, 0); + ASSERT(STATUS_SUCCESS == Result); + + Result = FspFsctlNotify(FileSystem->VolumeHandle, 0, 0); + ASSERT(STATUS_CANT_WAIT == Result); + + memfs_stop(memfs); +} + +static +void notify_abandon_test(void) +{ + if (WinFspDiskTests) + notify_abandon_dotest(MemfsDisk); + if (WinFspNetTests) + notify_abandon_dotest(MemfsNet); +} + +static +unsigned __stdcall notify_abandon_rename_dotest_thread(void *FilePath) +{ + FspDebugLog(__FUNCTION__ ": \"%S\"\n", FilePath); + + WCHAR NewFilePath[MAX_PATH]; + HANDLE Handle; + BOOL Success; + + Handle = CreateFileW(FilePath, + GENERIC_READ | GENERIC_WRITE, 0, 0, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, 0); + if (INVALID_HANDLE_VALUE == Handle) + return GetLastError(); + CloseHandle(Handle); + + StringCbPrintfW(NewFilePath, sizeof NewFilePath, L"%s.new", FilePath); + Success = MoveFileExW(FilePath, NewFilePath, 0); + + return Success ? 0 : GetLastError(); +} + +static +void notify_abandon_rename_dotest(ULONG Flags, PWSTR Prefix) +{ + void *memfs = memfs_start(Flags); + FSP_FILE_SYSTEM *FileSystem = MemfsFileSystem(memfs); + WCHAR FilePath[MAX_PATH]; + HANDLE Thread; + DWORD ExitCode; + NTSTATUS Result; + + Result = FspFsctlNotify(FileSystem->VolumeHandle, 0, 0); + ASSERT(STATUS_SUCCESS == Result); + + StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\file0", + Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs)); + + Thread = (HANDLE)_beginthreadex(0, 0, notify_abandon_rename_dotest_thread, FilePath, 0, 0); + ASSERT(0 != Thread); + + Sleep(1000); + + memfs_stop(memfs); + + WaitForSingleObject(Thread, INFINITE); + GetExitCodeThread(Thread, &ExitCode); + CloseHandle(Thread); + ASSERT(ERROR_OPERATION_ABORTED == ExitCode); +} + +static +void notify_abandon_rename_test(void) +{ + if (WinFspDiskTests) + notify_abandon_rename_dotest(MemfsDisk, 0); + if (WinFspNetTests) + notify_abandon_rename_dotest(MemfsNet, L"\\\\memfs\\share"); +} + +void notify_tests(void) +{ + if (!OptExternal) + { + TEST(notify_abandon_test); + TEST(notify_abandon_rename_test); + } +} diff --git a/tst/winfsp-tests/winfsp-tests.c b/tst/winfsp-tests/winfsp-tests.c index 9b0cf6a8..0f5aa0da 100644 --- a/tst/winfsp-tests/winfsp-tests.c +++ b/tst/winfsp-tests/winfsp-tests.c @@ -211,6 +211,7 @@ int main(int argc, char *argv[]) TESTSUITE(ea_tests); TESTSUITE(stream_tests); TESTSUITE(oplock_tests); + TESTSUITE(notify_tests); TESTSUITE(wsl_tests); TESTSUITE(volpath_tests);