fix BSOD occurred when detaching a busy device

Old plugout was prone to incur BSOD when a very busy device is detached.
Fiio device suffers from such the problem(see comment in #222). Indeed,
there was no locking between vusb I/O and plugout.
To resolve this issue, get/put for vusb has been introduced with a
reference count.

Author: cezanne

driver/vhci_ude/usbip_vhci_ude.vcxproj

@@ -47,6 +47,7 @@
     <ClCompile Include="vhci_queue_ep.c" />
     <ClCompile Include="vhci_urbr_store_status.c" />
     <ClCompile Include="vhci_urbr_store_vendor.c" />
+    <ClCompile Include="vhci_vusb.c" />
     <ClCompile Include="vhci_write.c" />
   </ItemGroup>
   <ItemGroup>
@@ -258,4 +259,4 @@
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>
-</Project>
+</Project>

driver/vhci_ude/usbip_vhci_ude.vcxproj.filters

@@ -123,6 +123,9 @@
     <ClCompile Include="vhci_urbr_store_reset.c">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="vhci_vusb.c">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <None Include="custom_wpp.ini" />

driver/vhci_ude/vhci_dbg.h

@@ -23,4 +23,4 @@ extern const char *dbg_urbr(purb_req_t urbr);
 
 #ifndef DBG
 const char *dbg_usbd_status(USBD_STATUS status);
-#endif
+#endif

driver/vhci_ude/vhci_dev.h

@@ -34,6 +34,9 @@ typedef struct _ctx_vusb
 	 */
 	BOOLEAN		is_simple_ep_alloc;
 	BOOLEAN		invalid;
+	/* reference count to prevent from removal while being used */
+	ULONG		refcnt;
+
 	// pending req which doesn't find an available urbr
 	WDFREQUEST	pending_req_read;
 	// a partially transferred urbr
@@ -90,15 +93,19 @@ typedef struct _ctx_safe_vusb
 {
 	pctx_vhci_t	vhci;
 	ULONG		port;
-	pctx_vusb_t	vusb;
 } ctx_safe_vusb_t, *pctx_safe_vusb_t;
 
 WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(ctx_safe_vusb_t, TO_SAFE_VUSB)
 
+#define TO_SAFE_VUSB_FROM_REQ(req)	TO_SAFE_VUSB(WdfRequestGetFileObject(req))
+
 #define VUSB_CREATING	((pctx_vusb_t)-1)
-#define VUSB_DELETING	((pctx_vusb_t)1)
-#define VUSB_IS_VALID(vusb)	((vusb) != NULL && (vusb) != VUSB_CREATING && (vusb) != VUSB_DELETING)
+#define VUSB_IS_VALID(vusb)	((vusb) != NULL && (vusb) != VUSB_CREATING && !(vusb)->invalid)
 
 extern NTSTATUS plugout_vusb(pctx_vhci_t vhci, CHAR port);
 
+extern pctx_vusb_t get_vusb(pctx_vhci_t vhci, ULONG port);
+extern pctx_vusb_t get_vusb_by_req(WDFREQUEST req);
+extern void put_vusb(pctx_vusb_t vusb);
+
 EXTERN_C_END

driver/vhci_ude/vhci_hc.c

@@ -61,31 +61,13 @@ create_fileobject(_In_ 	WDFDEVICE hdev, WDFREQUEST req, _In_ WDFFILEOBJECT fo)
 	TRD(VHCI, "Enter");
 
 	svusb->vhci = vhci;
-	svusb->vusb = NULL;
+	svusb->port = (ULONG)-1;
 
 	WdfRequestComplete(req, STATUS_SUCCESS);
 
 	TRD(VHCI, "Leave");
 }
 
-static VOID
-cleanup_fileobject(_In_ WDFFILEOBJECT fo)
-{
-	pctx_safe_vusb_t	svusb = TO_SAFE_VUSB(fo);
-
-	TRD(VHCI, "Enter");
-
-	/*
-	 * Not sure but the vusb maybe already be destroyed.
-	 * So after checked, proceed to plug out.
-	 */
-	if (svusb->vusb != NULL && svusb->vhci->vusbs[svusb->port] == svusb->vusb) {
-		plugout_vusb(svusb->vhci, (CHAR)svusb->port);
-	}
-
-	TRD(VHCI, "Leave");
-}
-
 static PAGEABLE VOID
 setup_fileobject(PWDFDEVICE_INIT dinit)
 {
@@ -95,7 +77,7 @@ setup_fileobject(PWDFDEVICE_INIT dinit)
 	PAGED_CODE();
 
 	WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&attrs, ctx_safe_vusb_t);
-	WDF_FILEOBJECT_CONFIG_INIT(&conf, create_fileobject, NULL, cleanup_fileobject);
+	WDF_FILEOBJECT_CONFIG_INIT(&conf, create_fileobject, NULL, NULL);
 	WdfDeviceInitSetFileObjectConfig(dinit, &conf, &attrs);
 }
 

driver/vhci_ude/vhci_ioctl.c

@@ -160,6 +160,27 @@ ioctl_plugout_vusb(WDFQUEUE queue, WDFREQUEST req, size_t inlen)
 	return plugout_vusb(vhci, port);
 }
 
+static NTSTATUS
+ioctl_shutdown_vusb(WDFQUEUE queue, WDFREQUEST req)
+{
+	pctx_vhci_t	vhci;
+	pctx_vusb_t	vusb;
+	NTSTATUS	status;
+
+	vusb = get_vusb_by_req(req);
+	if (vusb == NULL) {
+		/* already detached */
+		return STATUS_SUCCESS;
+	}
+
+	vhci = *TO_PVHCI(queue);
+
+	status = plugout_vusb(vhci, (CHAR)vusb->port);
+	put_vusb(vusb);
+
+	return status;
+}
+
 VOID
 io_device_control(_In_ WDFQUEUE queue, _In_ WDFREQUEST req,
 	_In_ size_t outlen, _In_ size_t inlen, _In_ ULONG ioctl_code)
@@ -183,6 +204,9 @@ io_device_control(_In_ WDFQUEUE queue, _In_ WDFREQUEST req,
 	case IOCTL_USBIP_VHCI_UNPLUG_HARDWARE:
 		status = ioctl_plugout_vusb(queue, req, inlen);
 		break;
+	case IOCTL_USBIP_VHCI_SHUTDOWN_HARDWARE:
+		status = ioctl_shutdown_vusb(queue, req);
+		break;
 	default:
 		if (UdecxWdfDeviceTryHandleUserIoctl((*TO_PVHCI(queue))->hdev, req)) {
 			TRD(IOCTL, "Leave: handled by Udecx");

driver/vhci_ude/vhci_plugin.c

@@ -94,13 +94,15 @@ setup_vusb(UDECXUSBDEVICE ude_usbdev, pvhci_pluginfo_t pluginfo)
 	}
 
 	vusb->devid = pluginfo->devid;
+	vusb->port = pluginfo->port;
 
 	vusb->ude_usbdev = ude_usbdev;
 	vusb->pending_req_read = NULL;
 	vusb->urbr_sent_partial = NULL;
 	vusb->len_sent_partial = 0;
 	vusb->seq_num = 0;
 	vusb->invalid = FALSE;
+	vusb->refcnt = 0;
 
 	if (vusb->iSerial > 0 && pluginfo->wserial[0] != L'\0')
 		vusb->wserial = libdrv_strdupW(pluginfo->wserial);
@@ -341,17 +343,9 @@ plugin_vusb(pctx_vhci_t vhci, WDFREQUEST req, pvhci_pluginfo_t pluginfo)
 
 	WdfSpinLockAcquire(vhci->spin_lock);
 	if (vusb != NULL) {
-		WDFFILEOBJECT	fo = WdfRequestGetFileObject(req);
-		if (fo != NULL) {
-			pctx_safe_vusb_t	svusb = TO_SAFE_VUSB(fo);
+		pctx_safe_vusb_t	svusb = TO_SAFE_VUSB_FROM_REQ(req);
 
-			svusb->vhci = vhci;
-			svusb->port = pluginfo->port;
-			svusb->vusb = vusb;
-		}
-		else {
-			TRE(PLUGIN, "empty fileobject. setup failed");
-		}
+		svusb->port = pluginfo->port;
 		status = STATUS_SUCCESS;
 	}
 	vhci->vusbs[pluginfo->port] = vusb;

driver/vhci_ude/vhci_plugout.c

@@ -50,27 +50,19 @@ abort_all_pending_urbrs(pctx_vusb_t vusb)
 	WdfSpinLockRelease(vusb->spin_lock);
 }
 
-static NTSTATUS
+static void
 vusb_plugout(pctx_vusb_t vusb)
 {
-	NTSTATUS	status;
-
 	/*
 	 * invalidate first to prevent requests from an upper layer.
 	 * If requests are consistently fed into a vusb about to be plugged out,
-	 * a live deadlock may occur where vusb aborts pending urbs indefinately. 
+	 * a live deadlock may occur where vusb aborts pending urbs indefinately.
 	 */
 	vusb->invalid = TRUE;
 	abort_pending_req_read(vusb);
 	abort_all_pending_urbrs(vusb);
 
-	status = UdecxUsbDevicePlugOutAndDelete(vusb->ude_usbdev);
-	if (NT_ERROR(status)) {
-		vusb->invalid = FALSE;
-		TRD(PLUGIN, "failed to plug out: %!STATUS!", status);
-		return status;
-	}
-	return STATUS_SUCCESS;
+	TRD(PLUGIN, "plugged out: port: %d", vusb->port);
 }
 
 static NTSTATUS
@@ -80,27 +72,16 @@ plugout_all_vusbs(pctx_vhci_t vhci)
 
 	TRD(PLUGIN, "plugging out all the devices!");
 
-	WdfSpinLockAcquire(vhci->spin_lock);
 	for (i = 0; i < vhci->n_max_ports; i++) {
-		NTSTATUS	status;
-		pctx_vusb_t	vusb = vhci->vusbs[i];
+		pctx_vusb_t	vusb;
+
+		vusb = get_vusb(vhci, i);
 		if (vusb == NULL)
 			continue;
 
-		vhci->vusbs[i] = VUSB_DELETING;
-		WdfSpinLockRelease(vhci->spin_lock);
-
-		status = vusb_plugout(vusb);
-
-		WdfSpinLockAcquire(vhci->spin_lock);
-		if (NT_ERROR(status)) {
-			vhci->vusbs[i] = vusb;
-			WdfSpinLockRelease(vhci->spin_lock);
-			return STATUS_UNSUCCESSFUL;
-		}
-		vhci->vusbs[i] = NULL;
+		vusb_plugout(vusb);
+		put_vusb(vusb);
 	}
-	WdfSpinLockRelease(vhci->spin_lock);
 
 	return STATUS_SUCCESS;
 }
@@ -109,36 +90,20 @@ NTSTATUS
 plugout_vusb(pctx_vhci_t vhci, CHAR port)
 {
 	pctx_vusb_t	vusb;
-	NTSTATUS	status;
 
 	if (port < 0)
 		return plugout_all_vusbs(vhci);
 
 	TRD(IOCTL, "plugging out device: port: %u", port);
 
-	WdfSpinLockAcquire(vhci->spin_lock);
-
-	vusb = vhci->vusbs[port];
+	vusb = get_vusb(vhci, port);
 	if (vusb == NULL) {
 		TRD(PLUGIN, "no matching vusb: port: %u", port);
-		WdfSpinLockRelease(vhci->spin_lock);
 		return STATUS_NO_SUCH_DEVICE;
 	}
 
-	vhci->vusbs[port] = VUSB_DELETING;
-	WdfSpinLockRelease(vhci->spin_lock);
-
-	status = vusb_plugout(vusb);
-
-	WdfSpinLockAcquire(vhci->spin_lock);
-	if (NT_ERROR(status)) {
-		vhci->vusbs[port] = vusb;
-		WdfSpinLockRelease(vhci->spin_lock);
-		return STATUS_UNSUCCESSFUL;
-	}
-	vhci->vusbs[port] = NULL;
-
-	WdfSpinLockRelease(vhci->spin_lock);
+	vusb_plugout(vusb);
+	put_vusb(vusb);
 
 	TRD(IOCTL, "completed to plug out: port: %u", port);
 

driver/vhci_ude/vhci_read.c

@@ -42,19 +42,19 @@ get_partial_urbr(pctx_vusb_t vusb)
 static VOID
 req_read_cancelled(WDFREQUEST req_read)
 {
-	pctx_safe_vusb_t	svusb;
 	pctx_vusb_t	vusb;
 
 	TRD(READ, "a pending read req cancelled");
 
-	svusb = TO_SAFE_VUSB(WdfRequestGetFileObject(req_read));
-	vusb = svusb->vusb;
-
-	WdfSpinLockAcquire(vusb->spin_lock);
-	if (vusb->pending_req_read == req_read) {
-		vusb->pending_req_read = NULL;
+	vusb = get_vusb_by_req(req_read);
+	if (vusb != NULL) {
+		WdfSpinLockAcquire(vusb->spin_lock);
+		if (vusb->pending_req_read == req_read) {
+			vusb->pending_req_read = NULL;
+		}
+		WdfSpinLockRelease(vusb->spin_lock);
+		put_vusb(vusb);
 	}
-	WdfSpinLockRelease(vusb->spin_lock);
 
 	WdfRequestComplete(req_read, STATUS_CANCELLED);
 }
@@ -132,23 +132,22 @@ read_vusb(pctx_vusb_t vusb, WDFREQUEST req)
 VOID
 io_read(_In_ WDFQUEUE queue, _In_ WDFREQUEST req, _In_ size_t len)
 {
-	pctx_safe_vusb_t	svusb;
 	pctx_vusb_t	vusb;
 	NTSTATUS	status;
 
 	UNREFERENCED_PARAMETER(queue);
 
 	TRD(READ, "Enter: len: %u", (ULONG)len);
 
-	svusb = TO_SAFE_VUSB(WdfRequestGetFileObject(req));
-	vusb = svusb->vusb;
-
-	if (vusb->invalid) {
-		TRD(READ, "vusb disconnected: port: %u", vusb->port);
+	vusb = get_vusb_by_req(req);
+	if (vusb == NULL) {
+		TRD(READ, "vusb disconnected: port: %u", TO_SAFE_VUSB_FROM_REQ(req)->port);
 		status = STATUS_DEVICE_NOT_CONNECTED;
 	}
-	else
+	else {
 		status = read_vusb(vusb, req);
+		put_vusb(vusb);
+	}
 
 	if (status != STATUS_PENDING) {
 		WdfRequestComplete(req, status);

driver/vhci_ude/vhci_vusb.c

@@ -0,0 +1,55 @@
+#include "vhci_driver.h"
+#include "vhci_vusb.tmh"
+
+pctx_vusb_t
+get_vusb(pctx_vhci_t vhci, ULONG port)
+{
+	pctx_vusb_t	vusb;
+
+	WdfSpinLockAcquire(vhci->spin_lock);
+
+	vusb = vhci->vusbs[port];
+	if (vusb == NULL || vusb->invalid) {
+		WdfSpinLockRelease(vhci->spin_lock);
+		return NULL;
+	}
+	vusb->refcnt++;
+
+	WdfSpinLockRelease(vhci->spin_lock);
+
+	return vusb;
+}
+
+pctx_vusb_t
+get_vusb_by_req(WDFREQUEST req)
+{
+	pctx_safe_vusb_t	svusb;
+
+	svusb = TO_SAFE_VUSB_FROM_REQ(req);
+	return get_vusb(svusb->vhci, svusb->port);
+}
+
+void
+put_vusb(pctx_vusb_t vusb)
+{
+	pctx_vhci_t	vhci = vusb->vhci;
+
+	WdfSpinLockAcquire(vhci->spin_lock);
+
+	ASSERT(vusb->refcnt > 0);
+	vusb->refcnt--;
+	if (vusb->refcnt == 0 && vusb->invalid) {
+		NTSTATUS	status;
+
+		vhci->vusbs[vusb->port] = NULL;
+		WdfSpinLockRelease(vhci->spin_lock);
+
+		status = UdecxUsbDevicePlugOutAndDelete(vusb->ude_usbdev);
+		if (NT_ERROR(status)) {
+			TRD(PLUGIN, "failed to plug out: %!STATUS!", status);
+		}
+	}
+	else {
+		WdfSpinLockRelease(vhci->spin_lock);
+	}
+}