Changeset 31323 in vbox for trunk/src/VBox

Timestamp:

Aug 2, 2010 6:18:22 PM (15 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

64354

Message:

SrvIntNetR0.cpp: Fixed lock order inversion in IntNetR0IfSetActive, must take the mutex before grabbing a (paranoid) busy reference.

File:

• trunk/src/VBox/Devices/Network/SrvIntNetR0.cpp (modified) (5 diffs)

trunk/src/VBox/Devices/Network/SrvIntNetR0.cpp

@@ -3738 +3738 @@
  *
  * This function will update the active interface count on the network and
- * activate or deactivate the trunk connection if necessary.  Note that in
- * order to do this it is necessary to abandond the network semaphore.
+ * activate or deactivate the trunk connection if necessary.
+ *
+ * The call must own the giant lock (we cannot take it here).
  *
  * @returns VBox status code.
@@ -3757 +3758 @@
      * to save us the extra hazzle.
      */
-    PINTNET         pIntNet = pNetwork->pIntNet;
-    int rc = RTSemMutexRequest(pIntNet->hMtxCreateOpenDestroy, RT_INDEFINITE_WAIT);
-    AssertRCReturn(rc, rc);
-
     PINTNETTRUNKIF  pTrunk  = NULL;
     RTSPINLOCKTMP   Tmp     = RTSPINLOCKTMP_INITIALIZER;
@@ -3806 +3803 @@
     /*
      * Tell the trunk if necessary.
+     * The wait for !busy is for the Solaris streams trunk driver (mostly).
      */
     if (pTrunk && pTrunk->pIfPort)
@@ -3814 +3812 @@
         pTrunk->pIfPort->pfnSetState(pTrunk->pIfPort, fActive ? INTNETTRUNKIFSTATE_ACTIVE : INTNETTRUNKIFSTATE_INACTIVE);
     }
-
-    RTSemMutexRelease(pIntNet->hMtxCreateOpenDestroy);
 
     return VINF_SUCCESS;
@@ -3851 +3847 @@
      * tricky there wrt to locking order.
      *
-     * Note! We mark the interface busy so the network cannot be removed while
-     *       we're working on it - paranoia strikes again.
-     */
-    intnetR0BusyIncIf(pIf);
-
-    int rc;
-    PINTNETNETWORK pNetwork = pIf->pNetwork;
-    if (pNetwork)
-        rc = intnetR0NetworkSetIfActive(pNetwork, pIf, fActive);
-    else
-        rc = VERR_WRONG_ORDER;
-
-    intnetR0BusyDecIf(pIf);
+     * 1. We take the giant lock here.  This makes sure nobody is re-enabling
+     *    the network while we're pausing it and vice versa.  This also enables
+     *    us to wait for the network to become idle before telling the trunk.
+     *    (Important on Solaris.)
+     *
+     * 2. For paranoid reasons, we grab a busy reference to the calling
+     *    interface.  This is totally unnecessary but should hurt (when done
+     *    after grabbing the giant lock).
+     */
+    int rc = RTSemMutexRequest(pIntNet->hMtxCreateOpenDestroy, RT_INDEFINITE_WAIT);
+    if (RT_SUCCESS(rc))
+    {
+        intnetR0BusyIncIf(pIf);
+
+        PINTNETNETWORK pNetwork = pIf->pNetwork;
+        if (pNetwork)
+            rc = intnetR0NetworkSetIfActive(pNetwork, pIf, fActive);
+        else
+            rc = VERR_WRONG_ORDER;
+
+        intnetR0BusyDecIf(pIf);
+        RTSemMutexRelease(pIntNet->hMtxCreateOpenDestroy);
+    }
+
     intnetR0IfRelease(pIf, pSession);
+    LogFlow(("IntNetR0IfSetActive: returns %Rrc\n", rc));
     return rc;
 }