dll,sys: fix 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.

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,

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);
     }
 }
 

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)
 {

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);

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;

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,6 +540,8 @@ VOID FspIoqStop(FSP_IOQ *Ioq)
     FspIoqEventSet(&Ioq->PendingIrpEvent);
         /* equivalent to FspIoqPendingResetSynch(Ioq) */
     KeReleaseSpinLock(&Ioq->SpinLock, Irql);
+    if (CancelIrps)
+    {
         PIRP Irp;
         while (0 != (Irp = IoCsqRemoveNextIrp(&Ioq->PendingIoCsq, 0)))
             Ioq->CompleteCanceledIrp(Irp);
@@ -548,6 +550,7 @@ VOID FspIoqStop(FSP_IOQ *Ioq)
         while (0 != (Irp = FspCsqRemoveNextIrp(&Ioq->RetriedIoCsq, 0)))
             Ioq->CompleteCanceledIrp(Irp);
     }
+}
 
 BOOLEAN FspIoqStopped(FSP_IOQ *Ioq)
 {

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)
@@ -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;
 }