Index: /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp
===================================================================
--- /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp	(revision 83564)
+++ /trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp	(revision 83565)
@@ -768,9 +768,11 @@
                               PVIRTIO_DESC_CHAIN_T pDescChain, REQ_RESP_HDR_T *pRespHdr, uint8_t *pbSense)
 {
+    /** @todo r=bird: There is too much allocating here and we'll leak stuff if
+     *        we're low on memory and one of the RTMemAllocZ calls fail! */
     uint8_t *pabSenseBuf = (uint8_t *)RTMemAllocZ(pThis->virtioScsiConfig.uSenseSize);
     AssertReturn(pabSenseBuf, VERR_NO_MEMORY);
 
     Log2Func(("   status: %s    response: %s\n",
-        SCSIStatusText(pRespHdr->uStatus), virtioGetReqRespText(pRespHdr->uResponse)));
+              SCSIStatusText(pRespHdr->uStatus), virtioGetReqRespText(pRespHdr->uResponse)));
 
     PRTSGBUF pReqSegBuf = (PRTSGBUF)RTMemAllocZ(sizeof(RTSGBUF));
@@ -791,10 +793,12 @@
 
     /* Copy segment data to malloc'd memory to avoid stack out-of-scope errors sanitizer doesn't detect */
+    /** @todo r=bird: The above comment makes zero sense as the memory is freed
+     *        before we return, so there cannot be any trouble with out-of-scope
+     *        stuff here. */
     for (int i = 0; i < 2; i++)
     {
         void *pv = paReqSegs[i].pvSeg;
-        paReqSegs[i].pvSeg = RTMemAlloc(paReqSegs[i].cbSeg);
+        paReqSegs[i].pvSeg = RTMemDup(pv, paReqSegs[i].cbSeg);
         AssertReturn(paReqSegs[i].pvSeg, VERR_NO_MEMORY);
-        memcpy(paReqSegs[i].pvSeg, pv, paReqSegs[i].cbSeg);
     }
 
@@ -930,6 +934,4 @@
     }
 
-    int cSegs = 0;
-
     if (   (VIRTIO_IS_IN_DIRECTION(pReq->enmTxDir)  && cbXfer32 > pReq->cbDataIn)
         || (VIRTIO_IS_OUT_DIRECTION(pReq->enmTxDir) && cbXfer32 > pReq->cbDataOut))
@@ -950,4 +952,6 @@
 
         /* req datain bytes already in guest phys mem. via virtioScsiIoReqCopyFromBuf() */
+        /** @todo r=bird: There is too much allocating here and we'll leak stuff if
+         *        we're low on memory and one of the RTMemAllocZ calls fail! */
 
         PRTSGBUF pReqSegBuf = (PRTSGBUF)RTMemAllocZ(sizeof(RTSGBUF));
@@ -957,4 +961,5 @@
         AssertReturn(paReqSegs, VERR_NO_MEMORY);
 
+        int cSegs = 0;
         paReqSegs[cSegs].pvSeg = &respHdr;
         paReqSegs[cSegs++].cbSeg = sizeof(respHdr);
@@ -964,10 +969,12 @@
 
         /* Copy segment data to malloc'd memory to avoid stack out-of-scope errors sanitizer doesn't detect */
+        /** @todo r=bird: The above comment makes zero sense as the memory is freed
+         *        before we return, so there cannot be any trouble with out-of-scope
+         *        stuff here. */
         for (int i = 0; i < cSegs; i++)
         {
             void *pv = paReqSegs[i].pvSeg;
-            paReqSegs[i].pvSeg = RTMemAlloc(paReqSegs[i].cbSeg);
+            paReqSegs[i].pvSeg = RTMemDup(pv, paReqSegs[i].cbSeg);
             AssertReturn(paReqSegs[i].pvSeg, VERR_NO_MEMORY);
-            memcpy(paReqSegs[i].pvSeg, pv, paReqSegs[i].cbSeg);
         }
 
@@ -1126,5 +1133,5 @@
     {
         LogRel(("* * * REPORT LUNS LU ACCESSED * * * "));
-        /* Force rejection. todo: figure out right way to handle. Note this is a very
+        /* Force rejection. */ /** @todo figure out right way to handle. Note this is a very
          * vague and confusing part of the VirtIO spec which deviates from the SCSI standard
          * I have not been able to determine how to implement this properly.  Guest drivers
@@ -1166,5 +1173,5 @@
         respHdr.uResidual  = cbDataIn + cbDataOut;
         virtioScsiR3ReqErr(pDevIns, pThis, pThisCC, qIdx, pDescChain, &respHdr , NULL);
-        RTMemFree(pVirtqReq);
+        RTMemFreeZ(pVirtqReq, cbReqHdr);
         return VINF_SUCCESS;
     }
@@ -1186,5 +1193,5 @@
         respHdr.uResidual  = cbDataOut + cbDataIn;
         virtioScsiR3ReqErr(pDevIns, pThis, pThisCC, qIdx, pDescChain, &respHdr, abSense);
-        RTMemFree(pVirtqReq);
+        RTMemFreeZ(pVirtqReq, cbReqHdr);
         return VINF_SUCCESS;
 
@@ -1204,5 +1211,5 @@
         respHdr.uResidual  = cbDataOut + cbDataIn;
         virtioScsiR3ReqErr(pDevIns, pThis, pThisCC, qIdx, pDescChain, &respHdr, abSense);
-        RTMemFree(pVirtqReq);
+        RTMemFreeZ(pVirtqReq, cbReqHdr);
         return VINF_SUCCESS;
     }
@@ -1218,5 +1225,5 @@
         respHdr.uResidual  = cbDataIn + cbDataOut;
         virtioScsiR3ReqErr(pDevIns, pThis, pThisCC, qIdx, pDescChain, &respHdr, NULL);
-        RTMemFree(pVirtqReq);
+        RTMemFreeZ(pVirtqReq, cbReqHdr);
         return VINF_SUCCESS;
     }
@@ -1237,5 +1244,5 @@
         respHdr.uResidual  = cbDataIn + cbDataOut;
         virtioScsiR3ReqErr(pDevIns, pThis, pThisCC, qIdx, pDescChain, &respHdr , abSense);
-        RTMemFree(pVirtqReq);
+        RTMemFreeZ(pVirtqReq, cbReqHdr);
         return VINF_SUCCESS;
     }
@@ -1253,5 +1260,5 @@
     if (RT_FAILURE(rc))
     {
-        RTMemFree(pVirtqReq);
+        RTMemFreeZ(pVirtqReq, cbReqHdr);
         AssertMsgRCReturn(rc, ("Failed to allocate I/O request, rc=%Rrc\n", rc), rc);
     }
@@ -1269,5 +1276,5 @@
     pReq->pbSense      = (uint8_t *)RTMemAllocZ(pReq->cbSenseAlloc);
     AssertMsgReturnStmt(pReq->pbSense, ("Out of memory allocating sense buffer"),
-                        virtioScsiR3FreeReq(pTarget, pReq); RTMemFree(pVirtqReq), VERR_NO_MEMORY);
+                        virtioScsiR3FreeReq(pTarget, pReq); RTMemFreeZ(pVirtqReq, cbReqHdr);, VERR_NO_MEMORY);
 
     /* Note: DrvSCSI allocates one virtual memory buffer for input and output phases of the request */
@@ -1309,5 +1316,5 @@
     }
 
-    RTMemFree(pVirtqReq);
+    RTMemFreeZ(pVirtqReq, cbReqHdr);
     return VINF_SUCCESS;
 }
@@ -1348,4 +1355,5 @@
     }
 
+    /** @todo r=bird: Memory leak here.  */
     AssertReturn(   (pScsiCtrlUnion->scsiCtrl.uType == VIRTIOSCSI_T_TMF
                      && pDescChain->cbPhysSend >= sizeof(VIRTIOSCSI_CTRL_TMF_T))
@@ -1354,6 +1362,9 @@
                      && pDescChain->cbPhysSend >= sizeof(VIRTIOSCSI_CTRL_AN_T)), 0);
 
+    /** @todo r=bird: This segment handling is unnecessary. A stack array should
+     *        suffice.  The stack variable w/ initializer + memcpy into paReqSegs
+     *        done in the switch below is also weird + suboptimal. */
     PRTSGSEG paReqSegs = (PRTSGSEG)RTMemAllocZ(sizeof(RTSGSEG) * 2);
-    AssertReturn(paReqSegs, VERR_NO_MEMORY);
+    AssertReturn(paReqSegs, VERR_NO_MEMORY); /** @todo r=bird: Memory leak here.  */
 
     switch (pScsiCtrlUnion->scsiCtrl.uType)
@@ -1492,13 +1503,15 @@
 
     PRTSGBUF pReqSegBuf = (PRTSGBUF)RTMemAllocZ(sizeof(RTSGBUF));
-    AssertReturn(pReqSegBuf, VERR_NO_MEMORY);
+    AssertReturn(pReqSegBuf, VERR_NO_MEMORY); /** @todo r=bird: Memory leak here.  */
 
     /* Copy segment data to malloc'd memory to avoid stack out-of-scope errors sanitizer doesn't detect */
+    /** @todo r=bird: The above comment makes zero sense as the memory is freed
+     *        before we return, so there cannot be any trouble with out-of-scope
+     *        stuff here. */
     for (int i = 0; i < cSegs; i++)
     {
         void *pv = paReqSegs[i].pvSeg;
-        paReqSegs[i].pvSeg = RTMemAlloc(paReqSegs[i].cbSeg);
-        AssertReturn(paReqSegs[i].pvSeg, VERR_NO_MEMORY);
-        memcpy(paReqSegs[i].pvSeg, pv, paReqSegs[i].cbSeg);
+        paReqSegs[i].pvSeg = RTMemDup(pv, paReqSegs[i].cbSeg);
+        AssertReturn(paReqSegs[i].pvSeg, VERR_NO_MEMORY); /** @todo r=bird: Memory leak here.  */
     }
 
@@ -1513,4 +1526,5 @@
     RTMemFree(paReqSegs);
     RTMemFree(pReqSegBuf);
+    /** @todo r=bird: Leaking pScsiCtrlUnion here! */
 
     return VINF_SUCCESS;