Changeset 97400 in vbox for trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp

Timestamp:

Nov 4, 2022 1:36:24 PM (2 years ago)

Author:

vboxsync

Message:

Devices/DevVirtioSCSI: Fix reference counting issue where a reference would've been released twice in the I/O request finish handler when the underlying driver reported a CHECK CONDITION error for the request causing the VM to hang when powering off, ticketref:21144

File:

• trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp (modified) (8 diffs)

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

@@ -804 +804 @@
 #endif
 
+/**
+ * Releases one reference from the given controller instances active request counter.
+ *
+ * @returns nothing.
+ * @param   pDevIns     The device instance.
+ * @param   pThis       VirtIO SCSI shared instance data.
+ * @param   pThisCC     VirtIO SCSI ring-3 instance data.
+ */
+DECLINLINE(void) virtioScsiR3Release(PPDMDEVINS pDevIns, PVIRTIOSCSI pThis, PVIRTIOSCSICC pThisCC)
+{
+    Assert(pThis->cActiveReqs);
+
+    if (!ASMAtomicDecU32(&pThis->cActiveReqs) && pThisCC->fQuiescing)
+        PDMDevHlpAsyncNotificationCompleted(pDevIns);
+}
+
+/**
+ * Retains one reference for the given controller instances active request counter.
+ *
+ * @returns nothing.
+ * @param   pThis       VirtIO SCSI shared instance data.
+ */
+DECLINLINE(void) virtioScsiR3Retain(PVIRTIOSCSI pThis)
+{
+    ASMAtomicIncU32(&pThis->cActiveReqs);
+}
+
 /** Internal worker. */
 static void virtioScsiR3FreeReq(PVIRTIOSCSITARGET pTarget, PVIRTIOSCSIREQ pReq)
@@ -865 +892 @@
     virtioCoreR3VirtqUsedBufPut(pDevIns, &pThis->Virtio, uVirtqNbr, &ReqSgBuf, pVirtqBuf, true /* fFence */);
     virtioCoreVirtqUsedRingSync(pDevIns, &pThis->Virtio, uVirtqNbr);
-
-    if (!ASMAtomicDecU32(&pThis->cActiveReqs) && pThisCC->fQuiescing)
-        PDMDevHlpAsyncNotificationCompleted(pDevIns);
 
     Log2(("---------------------------------------------------------------------------------\n"));
@@ -1059 +1083 @@
 
     virtioScsiR3FreeReq(pTarget, pReq);
-
-    if (!ASMAtomicDecU32(&pThis->cActiveReqs) && pThisCC->fQuiescing)
-        PDMDevHlpAsyncNotificationCompleted(pDevIns);
-
+    virtioScsiR3Release(pDevIns, pThis, pThisCC);
     return rc;
 }
@@ -1156 +1177 @@
                                  uint16_t uVirtqNbr, PVIRTQBUF pVirtqBuf)
 {
-
-    ASMAtomicIncU32(&pThis->cActiveReqs);
-
     /*
      * Validate configuration values we use here before we start.
@@ -1295 +1313 @@
      * Have underlying driver allocate a req of size set during initialization of this device.
      */
+    virtioScsiR3Retain(pThis);
+
     PDMMEDIAEXIOREQ     hIoReq    = NULL;
     PVIRTIOSCSIREQ      pReq      = NULL;
@@ -1302 +1322 @@
                                       PDMIMEDIAEX_F_SUSPEND_ON_RECOVERABLE_ERR);
 
-    AssertMsgRCReturn(rc, ("Failed to allocate I/O request, rc=%Rrc\n", rc), rc);
+    if (RT_FAILURE(rc))
+    {
+        virtioScsiR3Release(pDevIns, pThis, pThisCC);
+        return rc;
+    }
 
     pReq->hIoReq      = hIoReq;
@@ -1355 +1379 @@
         virtioScsiR3ReqErr(pDevIns, pThis, pThisCC, uVirtqNbr, pVirtqBuf, &respHdr, abSense, cbSenseCfg);
         virtioScsiR3FreeReq(pTarget, pReq);
+        virtioScsiR3Release(pDevIns, pThis, pThisCC);
     }
     return VINF_SUCCESS;
@@ -2393 +2418 @@
         {
             /* Stop considering this request active */
-            if (!ASMAtomicDecU32(&pThis->cActiveReqs) && pThisCC->fQuiescing)
-                PDMDevHlpAsyncNotificationCompleted(pDevIns);
+            virtioScsiR3Release(pDevIns, pThis, pThisCC);
             break;
         }
         case PDMMEDIAEXIOREQSTATE_ACTIVE:
-            ASMAtomicIncU32(&pThis->cActiveReqs);
+            virtioScsiR3Retain(pThis);
             break;
         default: