diff --git a/inc/winfsp/fsctl.h b/inc/winfsp/fsctl.h index 9d2a28fe..d1386c99 100644 --- a/inc/winfsp/fsctl.h +++ b/inc/winfsp/fsctl.h @@ -85,6 +85,8 @@ extern const __declspec(selectany) GUID FspFsvrtDeviceClassGuid = CTL_CODE(FILE_DEVICE_FILE_SYSTEM, 0x800 + 't', METHOD_OUT_DIRECT, FILE_ANY_ACCESS) #define FSP_FSCTL_STOP \ CTL_CODE(FILE_DEVICE_FILE_SYSTEM, 0x800 + 'S', METHOD_BUFFERED, FILE_ANY_ACCESS) +#define FSP_FSCTL_STOP0 \ + CTL_CODE(FILE_DEVICE_FILE_SYSTEM, 0x800 + 's', METHOD_BUFFERED, FILE_ANY_ACCESS) #define FSP_FSCTL_NOTIFY \ CTL_CODE(FILE_DEVICE_FILE_SYSTEM, 0x800 + 'n', METHOD_NEITHER, FILE_ANY_ACCESS) @@ -647,6 +649,7 @@ FSP_API NTSTATUS FspFsctlTransact(HANDLE VolumeHandle, PVOID RequestBuf, SIZE_T *PRequestBufSize, BOOLEAN Batch); FSP_API NTSTATUS FspFsctlStop(HANDLE VolumeHandle); +FSP_API NTSTATUS FspFsctlStop0(HANDLE VolumeHandle); FSP_API NTSTATUS FspFsctlNotify(HANDLE VolumeHandle, FSP_FSCTL_NOTIFY_INFO *NotifyInfo, SIZE_T Size); FSP_API NTSTATUS FspFsctlGetVolumeList(PWSTR DevicePath, diff --git a/src/dll/fs.c b/src/dll/fs.c index d0d525bc..9527e8f1 100644 --- a/src/dll/fs.c +++ b/src/dll/fs.c @@ -349,7 +349,7 @@ exit: FspFileSystemSetDispatcherResult(FileSystem, Result); - FspFsctlStop(FileSystem->VolumeHandle); + FspFsctlStop0(FileSystem->VolumeHandle); if (0 != DispatcherThread) { @@ -398,11 +398,13 @@ FSP_API VOID FspFileSystemStopDispatcher(FSP_FILE_SYSTEM *FileSystem) if (0 == FileSystem->DispatcherThread) return; - FspFsctlStop(FileSystem->VolumeHandle); + FspFsctlStop0(FileSystem->VolumeHandle); WaitForSingleObject(FileSystem->DispatcherThread, INFINITE); CloseHandle(FileSystem->DispatcherThread); FileSystem->DispatcherThread = 0; + + FspFsctlStop(FileSystem->VolumeHandle); } FSP_API VOID FspFileSystemSendResponse(FSP_FILE_SYSTEM *FileSystem, @@ -423,7 +425,7 @@ FSP_API VOID FspFileSystemSendResponse(FSP_FILE_SYSTEM *FileSystem, { FspFileSystemSetDispatcherResult(FileSystem, Result); - FspFsctlStop(FileSystem->VolumeHandle); + FspFsctlStop0(FileSystem->VolumeHandle); } } diff --git a/src/dll/fsctl.c b/src/dll/fsctl.c index 9fad2ae1..60664cfa 100644 --- a/src/dll/fsctl.c +++ b/src/dll/fsctl.c @@ -161,6 +161,16 @@ FSP_API NTSTATUS FspFsctlStop(HANDLE VolumeHandle) return STATUS_SUCCESS; } +FSP_API NTSTATUS FspFsctlStop0(HANDLE VolumeHandle) +{ + DWORD Bytes; + + if (!DeviceIoControl(VolumeHandle, FSP_FSCTL_STOP0, 0, 0, 0, 0, &Bytes, 0)) + return FspNtStatusFromWin32(GetLastError()); + + return STATUS_SUCCESS; +} + FSP_API NTSTATUS FspFsctlNotify(HANDLE VolumeHandle, FSP_FSCTL_NOTIFY_INFO *NotifyInfo, SIZE_T Size) { diff --git a/src/sys/driver.h b/src/sys/driver.h index 79536a0a..38780e22 100644 --- a/src/sys/driver.h +++ b/src/sys/driver.h @@ -946,7 +946,7 @@ NTSTATUS FspIoqCreate( ULONG IrpCapacity, PLARGE_INTEGER IrpTimeout, VOID (*CompleteCanceledIrp)(PIRP Irp), FSP_IOQ **PIoq); VOID FspIoqDelete(FSP_IOQ *Ioq); -VOID FspIoqStop(FSP_IOQ *Ioq); +VOID FspIoqStop(FSP_IOQ *Ioq, BOOLEAN CancelIrps); BOOLEAN FspIoqStopped(FSP_IOQ *Ioq); VOID FspIoqRemoveExpired(FSP_IOQ *Ioq, UINT64 InterruptTime); BOOLEAN FspIoqPostIrpEx(FSP_IOQ *Ioq, PIRP Irp, BOOLEAN BestEffort, NTSTATUS *PResult); diff --git a/src/sys/fsctl.c b/src/sys/fsctl.c index b6b18cc9..80d3f51e 100644 --- a/src/sys/fsctl.c +++ b/src/sys/fsctl.c @@ -95,6 +95,28 @@ static NTSTATUS FspFsctlFileSystemControl( Result = FspVolumeTransact(FsctlDeviceObject, Irp, IrpSp); break; case FSP_FSCTL_STOP: + case FSP_FSCTL_STOP0: + /* Fix GitHub issue #369 + * + * The original WinFsp protocol for shutting down a file system was to issue + * an FSP_FSCTL_STOP control code to the fsctl device. This would set the IOQ + * to the "stopped" state and would also cancel all active IRP's. Cancelation + * of IRP's would sometimes free buffers that may have still been in use by + * the user mode file system threads; hence access violation. + * + * To fix this problem a new control code FSP_FSCTL_STOP0 is introduced. The + * new file system shutdown protocol is backwards compatible with the original + * one and works as follows: + * + * - First the file system process issues an FSP_FSCTL_STOP0 control code which + * sets the IOQ to the "stopped" state but does NOT cancel IRP's. + * + * - Then the file system process waits for its dispatcher threads to complete + * (see FspFileSystemStopDispatcher). + * + * - Finally the file system process issues an FSP_FSCTL_STOP control code + * which stops the (already stopped) IOQ and cancels all IRP's. + */ if (0 != IrpSp->FileObject->FsContext2) Result = FspVolumeStop(FsctlDeviceObject, Irp, IrpSp); break; diff --git a/src/sys/ioq.c b/src/sys/ioq.c index f5d58e24..dbab2390 100644 --- a/src/sys/ioq.c +++ b/src/sys/ioq.c @@ -526,12 +526,12 @@ NTSTATUS FspIoqCreate( VOID FspIoqDelete(FSP_IOQ *Ioq) { - FspIoqStop(Ioq); + FspIoqStop(Ioq, TRUE); FspIoqEventFinalize(&Ioq->PendingIrpEvent); FspFree(Ioq); } -VOID FspIoqStop(FSP_IOQ *Ioq) +VOID FspIoqStop(FSP_IOQ *Ioq, BOOLEAN CancelIrps) { KIRQL Irql; KeAcquireSpinLock(&Ioq->SpinLock, &Irql); @@ -540,13 +540,16 @@ VOID FspIoqStop(FSP_IOQ *Ioq) FspIoqEventSet(&Ioq->PendingIrpEvent); /* equivalent to FspIoqPendingResetSynch(Ioq) */ KeReleaseSpinLock(&Ioq->SpinLock, Irql); - PIRP Irp; - while (0 != (Irp = IoCsqRemoveNextIrp(&Ioq->PendingIoCsq, 0))) - Ioq->CompleteCanceledIrp(Irp); - while (0 != (Irp = FspCsqRemoveNextIrp(&Ioq->ProcessIoCsq, 0))) - Ioq->CompleteCanceledIrp(Irp); - while (0 != (Irp = FspCsqRemoveNextIrp(&Ioq->RetriedIoCsq, 0))) - Ioq->CompleteCanceledIrp(Irp); + if (CancelIrps) + { + PIRP Irp; + while (0 != (Irp = IoCsqRemoveNextIrp(&Ioq->PendingIoCsq, 0))) + Ioq->CompleteCanceledIrp(Irp); + while (0 != (Irp = FspCsqRemoveNextIrp(&Ioq->ProcessIoCsq, 0))) + Ioq->CompleteCanceledIrp(Irp); + while (0 != (Irp = FspCsqRemoveNextIrp(&Ioq->RetriedIoCsq, 0))) + Ioq->CompleteCanceledIrp(Irp); + } } BOOLEAN FspIoqStopped(FSP_IOQ *Ioq) diff --git a/src/sys/volume.c b/src/sys/volume.c index 3c033683..432e712a 100644 --- a/src/sys/volume.c +++ b/src/sys/volume.c @@ -410,7 +410,7 @@ static VOID FspVolumeDeleteNoLock( IrpSp->FileObject->FsContext2 = 0; /* stop the I/O queue */ - FspIoqStop(FsvolDeviceExtension->Ioq); + FspIoqStop(FsvolDeviceExtension->Ioq, TRUE); /* do we have a virtual disk device or are we registered with fsmup? */ if (0 != FsvolDeviceExtension->FsvrtDeviceObject) @@ -1001,7 +1001,7 @@ NTSTATUS FspVolumeTransact( if (!FspFsctlTransactCanProduceRequest(Request, BufferEnd)) break; } - + if (0 >= LoopCount--) /* upper bound on loop guarantees forward progress! */ break; @@ -1056,13 +1056,37 @@ NTSTATUS FspVolumeStop( ASSERT(IRP_MJ_FILE_SYSTEM_CONTROL == IrpSp->MajorFunction); ASSERT(IRP_MN_USER_FS_REQUEST == IrpSp->MinorFunction); - ASSERT(FSP_FSCTL_STOP == IrpSp->Parameters.FileSystemControl.FsControlCode); + ASSERT( + FSP_FSCTL_STOP == IrpSp->Parameters.FileSystemControl.FsControlCode || + FSP_FSCTL_STOP0 == IrpSp->Parameters.FileSystemControl.FsControlCode); ASSERT(0 != IrpSp->FileObject->FsContext2); PDEVICE_OBJECT FsvolDeviceObject = IrpSp->FileObject->FsContext2; FSP_FSVOL_DEVICE_EXTENSION *FsvolDeviceExtension = FspFsvolDeviceExtension(FsvolDeviceObject); - FspIoqStop(FsvolDeviceExtension->Ioq); + /* Fix GitHub issue #369 + * + * The original WinFsp protocol for shutting down a file system was to issue + * an FSP_FSCTL_STOP control code to the fsctl device. This would set the IOQ + * to the "stopped" state and would also cancel all active IRP's. Cancelation + * of IRP's would sometimes free buffers that may have still been in use by + * the user mode file system threads; hence access violation. + * + * To fix this problem a new control code FSP_FSCTL_STOP0 is introduced. The + * new file system shutdown protocol is backwards compatible with the original + * one and works as follows: + * + * - First the file system process issues an FSP_FSCTL_STOP0 control code which + * sets the IOQ to the "stopped" state but does NOT cancel IRP's. + * + * - Then the file system process waits for its dispatcher threads to complete + * (see FspFileSystemStopDispatcher). + * + * - Finally the file system process issues an FSP_FSCTL_STOP control code + * which stops the (already stopped) IOQ and cancels all IRP's. + */ + FspIoqStop(FsvolDeviceExtension->Ioq, + FSP_FSCTL_STOP == IrpSp->Parameters.FileSystemControl.FsControlCode); return STATUS_SUCCESS; }