Changeset 52878 in vbox for trunk/src/VBox

Timestamp:

Sep 28, 2014 5:05:33 PM (10 years ago)

Author:

vboxsync

Message:

USB: Fix lock order violation when canceling all URBs

Location:

trunk/src/VBox/Devices/USB

Files:

• DevOHCI.cpp (modified) (10 diffs)
• DrvVUSBRootHub.cpp (modified) (2 diffs)

trunk/src/VBox/Devices/USB/DevOHCI.cpp

@@ -371 +371 @@
     /** Event semaphore to interact with the framer thread. */
     R3PTRTYPE(RTSEMEVENT) hSemEventFrame;
+    /** Event semaphore to release the thread waiting for the framer thread to stop. */
+    R3PTRTYPE(RTSEMEVENTMULTI) hSemEventFrameStopped;
     /** Flag whether the framer thread should processing frames. */
     volatile bool         fBusStarted;
@@ -793 +795 @@
 static void                 rhport_power(POHCIROOTHUB pRh, unsigned iPort, bool fPowerUp);
 static void                 ohciBusResume(POHCI ohci, bool fHardware);
+static void                 ohciBusStop(POHCI pThis);
 
 static DECLCALLBACK(void)   ohciRhXferCompletion(PVUSBIROOTHUBPORT pInterface, PVUSBURB pUrb);
@@ -1111 +1114 @@
          fResetOnLinux ? " (reset on linux)" : ""));
 
+    /* Stop the bus in any case, disabling walking the lists. */
+    ohciBusStop(pThis);
+
     /*
      * Cancel all outstanding URBs.
@@ -1127 +1133 @@
     else
         pThis->ctl &= OHCI_CTL_IR | OHCI_CTL_RWC;       /* IR and RWC are preserved on software reset. */
+
+    /* Clear the HCFS bits first to make setting the new state work. */
+    pThis->ctl &= ~OHCI_CTL_HCFS;
     pThis->ctl |= fNewMode;
     pThis->status = 0;
@@ -3836 +3845 @@
                && pThread->enmState == PDMTHREADSTATE_RUNNING
                && RT_SUCCESS(rc))
+        {
+            /* Signal the waiter that we are stopped now. */
+            rc = RTSemEventMultiSignal(pThis->hSemEventFrameStopped);
+            AssertRC(rc);
             rc = RTSemEventWait(pThis->hSemEventFrame, RT_INDEFINITE_WAIT);
+        }
 
         AssertLogRelMsgReturn(RT_SUCCESS(rc) || rc == VERR_TIMEOUT, ("%Rrc\n", rc), rc);
@@ -3905 +3919 @@
 
     pThis->SofTime = PDMDevHlpTMTimeVirtGet(pThis->CTX_SUFF(pDevIns));
-    ASMAtomicXchgBool(&pThis->fBusStarted, true);
-    RTSemEventSignal(pThis->hSemEventFrame);
+    bool fBusActive = ASMAtomicXchgBool(&pThis->fBusStarted, true);
+    if (!fBusActive)
+        RTSemEventSignal(pThis->hSemEventFrame);
 }
 
@@ -3914 +3929 @@
 static void ohciBusStop(POHCI pThis)
 {
-    ASMAtomicXchgBool(&pThis->fBusStarted, false);
-    RTSemEventSignal(pThis->hSemEventFrame);
+    bool fBusActive = ASMAtomicXchgBool(&pThis->fBusStarted, false);
+    if (fBusActive)
+    {
+        int rc = RTSemEventMultiReset(pThis->hSemEventFrameStopped);
+        AssertRC(rc);
+
+        /* Signal the frame thread to stop. */
+        RTSemEventSignal(pThis->hSemEventFrame);
+
+        /* Wait for signal from the thrad that it stopped. */
+        rc = RTSemEventMultiWait(pThis->hSemEventFrameStopped, RT_INDEFINITE_WAIT);
+        AssertRC(rc);
+    }
     VUSBIDevPowerOff(pThis->RootHub.pIDev);
 }
@@ -5516 +5542 @@
      *            just one way of getting into the UsbReset state.
      */
-    ohciBusStop(pThis);
     ohciDoReset(pThis, OHCI_USB_RESET, true /* reset devices */);
 }
@@ -5633 +5658 @@
      * Destroy event sempahores.
      */
-    RTSemEventDestroy(pThis->hSemEventFrame);
+    if (pThis->hSemEventFrame)
+        RTSemEventDestroy(pThis->hSemEventFrame);
+    if (pThis->hSemEventFrameStopped)
+        RTSemEventMultiDestroy(pThis->hSemEventFrameStopped);
     if (RTCritSectIsInitialized(&pThis->CritSect))
         RTCritSectDelete(&pThis->CritSect);
@@ -5789 +5817 @@
 
     rc = RTSemEventCreate(&pThis->hSemEventFrame);
+    AssertRCReturn(rc, rc);
+
+    rc = RTSemEventMultiCreate(&pThis->hSemEventFrameStopped);
     AssertRCReturn(rc, rc);
 

trunk/src/VBox/Devices/USB/DrvVUSBRootHub.cpp

@@ -609 +609 @@
     /*
      * Cancel the URBS.
-     */
-    RTCritSectEnter(&pDev->CritSectAsyncUrbs);
+     *
+     * Not using th CritAsyncUrbs critical section here is safe
+     * as the I/O thread is the only thread accessing this struture at the
+     * moment.
+     */
     PVUSBURB pUrb = pDev->pAsyncUrbHead;
 
@@ -620 +623 @@
         pUrb = pNext;
     }
-    RTCritSectLeave(&pDev->CritSectAsyncUrbs);
 
     return VINF_SUCCESS;