Changeset 52434 in vbox for trunk/src/VBox/Devices

Timestamp:

Aug 20, 2014 8:09:10 PM (10 years ago)

Author:

vboxsync

Message:

Storage/AHCI: Fix tight race when the guest resets a port leading to an inconsistent active request counter and a release assertion later on. The release assertion must be removed though because it might trigger by mistake now if the request counter is 0 due to thread preemption

File:

• trunk/src/VBox/Devices/Storage/DevAHCI.cpp (modified) (3 diffs)

trunk/src/VBox/Devices/Storage/DevAHCI.cpp

@@ -5718 +5718 @@
     }
 
-    AssertRelease(   !ASMAtomicReadU32(&pAhciPort->cTasksActive)
-                  || (pAhciReqExcept && ASMAtomicReadU32(&pAhciPort->cTasksActive) == 1));
     return true; /* always true for now because tasks don't use guest memory as the buffer which makes canceling a task impossible. */
 }
@@ -5979 +5977 @@
     }
 
+    bool fPortReset = ASMAtomicReadBool(&pAhciPort->fPortReset);
     fXchg = ASMAtomicCmpXchgU32((volatile uint32_t *)&pAhciReq->enmTxState, AHCITXSTATE_FREE, AHCITXSTATE_ACTIVE);
 
-    if (   fXchg
-        && !ASMAtomicReadBool(&pAhciPort->fPortReset))
+    if (fXchg && !fPortReset)
     {
         if (pAhciReq->enmTxDir == AHCITXDIR_READ)
@@ -6097 +6095 @@
          * The guest might use it for other things now because it doesn't know about that task anymore.
          */
-        AssertMsg(   pAhciReq->enmTxState == AHCITXSTATE_CANCELED
-                  || pAhciPort->fPortReset,
-                  ("Task is not active but wasn't canceled!\n"));
+        AssertMsg(pAhciReq->enmTxState == AHCITXSTATE_CANCELED || fPortReset,
+                  ("Task is not active but wasn't canceled and no port reset is active!\n"));
+
+        /*
+         * If this handler switched the request state from active to free the request counter
+         * must be decremented.
+         */
+        if (fXchg)
+        {
+            Assert(fPortReset);
+            AssertReleaseMsg(ASMAtomicReadU32(&pAhciPort->cTasksActive) > 0 ,
+                             ("Inconsistent request counter\n"));
+            ASMAtomicDecU32(&pAhciPort->cTasksActive);
+        }
 
         fCanceled = true;