Changeset 37083 in vbox

Timestamp:

May 13, 2011 7:24:39 PM (14 years ago)

Author:

vboxsync

Message:

usb: prevent the monitor from being unload, when unhook fails

Location:

trunk/src/VBox/HostDrivers/VBoxUSB/win/mon

Files:

• VBoxUsbHook.cpp (modified) (1 diff)
• VBoxUsbMon.cpp (modified) (6 diffs)

trunk/src/VBox/HostDrivers/VBoxUSB/win/mon/VBoxUsbHook.cpp

@@ -51 +51 @@
     if (pfnOldVal != pHook->pfnHook)
     {
-        AssertFailed();
+        AssertMsgFailed(("unhook failed!!!\n"));
         /* this is bad! this could happen if someone else has chained another hook,
-         * return the failure and don't do anything else */
+         * or (which is even worse) restored the "initial" entry value it saved when doing a hooking before us
+         * return the failure and don't do anything else
+         * the best thing to do if this happens is to leave everything as is
+         * and to prevent the driver from being unloaded to ensure no one references our unloaded hook routine */
         KeReleaseSpinLock(&pHook->Lock, Irql);
         return STATUS_UNSUCCESSFUL;

trunk/src/VBox/HostDrivers/VBoxUSB/win/mon/VBoxUsbMon.cpp

@@ -54 +54 @@
 {
     VBOXUSBHOOK_ENTRY Hook;
+    bool fUninitFailed;
 } VBOXUSBHUB_PNPHOOK, *PVBOXUSBHUB_PNPHOOK;
 
@@ -68 +69 @@
     IO_REMOVE_LOCK RmLock;
     uint32_t cOpens;
+    volatile LONG ulPreventUnloadOn;
+    PFILE_OBJECT pPreventUnloadFileObj;
 } VBOXUSBMONGLOBALS, *PVBOXUSBMONGLOBALS;
 
@@ -657 +660 @@
     return STATUS_SUCCESS;
 #else
+    if (g_VBoxUsbMonGlobals.UsbHubPnPHook.fUninitFailed)
+    {
+        AssertMsgFailed(("trying to hook usbhub pnp after the unhook failed, do nothing & pretend success..\n"));
+        return STATUS_SUCCESS;
+    }
     return VBoxUsbHookInstall(&g_VBoxUsbMonGlobals.UsbHubPnPHook.Hook);
 #endif
@@ -666 +674 @@
     return STATUS_SUCCESS;
 #else
-    return VBoxUsbHookUninstall(&g_VBoxUsbMonGlobals.UsbHubPnPHook.Hook);
+    NTSTATUS Status = VBoxUsbHookUninstall(&g_VBoxUsbMonGlobals.UsbHubPnPHook.Hook);
+    if (!NT_SUCCESS(Status))
+    {
+        AssertMsgFailed(("usbhub pnp unhook failed, setting the fUninitFailed flag, the current value of fUninitFailed (%d)\n", g_VBoxUsbMonGlobals.UsbHubPnPHook.fUninitFailed));
+        g_VBoxUsbMonGlobals.UsbHubPnPHook.fUninitFailed = true;
+    }
+    return Status;
 #endif
 }
@@ -677 +691 @@
             FALSE, /* BOOLEAN Alertable */
             NULL /* IN PLARGE_INTEGER Timeout */
-        );
-    Assert(Status == STATUS_SUCCESS);
-    if (Status == STATUS_SUCCESS)
-    {
-        do
-        {
-            if (--g_VBoxUsbMonGlobals.cOpens)
-                break;
-
-            Status = vboxUsbMonHookUninstall();
-            if (NT_SUCCESS(Status))
-            {
-                Status = VBoxUsbFltTerm();
-                if (NT_SUCCESS(Status))
-                {
-                    Status = STATUS_SUCCESS;
-                    break;
-                }
-                else
-                {
-                    AssertFailed();
-                }
-            }
-            else
-            {
-                AssertFailed();
-            }
-
-            ++g_VBoxUsbMonGlobals.cOpens;
-            Assert(g_VBoxUsbMonGlobals.cOpens == 1);
-        } while (0);
-
-        KeSetEvent(&g_VBoxUsbMonGlobals.OpenSynchEvent, 0, FALSE);
-    }
+            );
+    AssertRelease(Status == STATUS_SUCCESS);
+
+    do
+    {
+        if (--g_VBoxUsbMonGlobals.cOpens)
+            break;
+
+        Status = vboxUsbMonHookUninstall();
+
+        NTSTATUS tmpStatus = VBoxUsbFltTerm();
+        if (!NT_SUCCESS(tmpStatus))
+        {
+            /* this means a driver state is screwed up, KeBugCheckEx here ? */
+            AssertReleaseFailed();
+        }
+    } while (0);
+
+    KeSetEvent(&g_VBoxUsbMonGlobals.OpenSynchEvent, 0, FALSE);
+
     return Status;
 }
@@ -818 +818 @@
     NTSTATUS Status = vboxUsbMonContextClose(pCtx);
     Assert(Status == STATUS_SUCCESS);
+    if (Status != STATUS_SUCCESS)
+    {
+        AssertMsgFailed(("close failed with Status 0x%x, prefent unload\n", Status));
+        if (!InterlockedExchange(&g_VBoxUsbMonGlobals.ulPreventUnloadOn, 1))
+        {
+            LogRel(("ulPreventUnloadOn not set, preventing unload\n"));
+            UNICODE_STRING UniName;
+            PDEVICE_OBJECT pTmpDevObj;
+            RtlInitUnicodeString(&UniName, USBMON_DEVICE_NAME_NT);
+            NTSTATUS tmpStatus = IoGetDeviceObjectPointer(&UniName, FILE_ALL_ACCESS, &g_VBoxUsbMonGlobals.pPreventUnloadFileObj, &pTmpDevObj);
+            AssertRelease(NT_SUCCESS(tmpStatus));
+            AssertRelease(pTmpDevObj == pDevObj);
+        }
+        else
+        {
+            AssertMsgFailed(("ulPreventUnloadOn already set\n"));
+        }
+        Status = STATUS_SUCCESS;
+    }
     pFileObj->FsContext = NULL;
     pIrp->IoStatus.Status = Status;