Changeset 51750 in vbox for trunk/src/VBox/Storage

Timestamp:

Jun 27, 2014 8:44:58 PM (11 years ago)

Author:

vboxsync

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

94562

Message:

Storage: Fix concurrency issue with filters configured which can result in corrupt data being transfered, the filters must be applied under the lock to avoid multiple threads accessing the same filter at the same time

File:

• trunk/src/VBox/Storage/VD.cpp (modified) (9 diffs)

trunk/src/VBox/Storage/VD.cpp

@@ -480 +480 @@
  * it was alloacted elsewhere (stack, ...). */
 #define VDIOCTX_FLAGS_DONT_FREE              RT_BIT_32(4)
-/* Don't set the modified flag for this I/O context when writing. */
+/** Don't set the modified flag for this I/O context when writing. */
 #define VDIOCTX_FLAGS_DONT_SET_MODIFIED_FLAG RT_BIT_32(5)
+/** The write filter was applied already and shouldn't be applied a second time.
+ * Used at the beginning of vdWriteHelperAsync() because it might be called
+ * multiple times.
+ */
+#define VDIOCTX_FLAGS_WRITE_FILTER_APPLIED   RT_BIT_32(6)
 
 /** NIL I/O context pointer value. */
@@ -967 +972 @@
     int rc = VINF_SUCCESS;
 
+    VD_IS_LOCKED(pDisk);
+
     if (pDisk->pFilterHead)
     {
@@ -998 +1005 @@
 {
     int rc = VINF_SUCCESS;
+
+    VD_IS_LOCKED(pDisk);
 
     if (pDisk->pFilterHead)
@@ -1741 +1750 @@
         if (pTmp == pIoCtxRc)
         {
+            if (   rcTmp == VINF_VD_ASYNC_IO_FINISHED
+                && RT_SUCCESS(pTmp->rcReq)
+                && pTmp->enmTxDir == VDIOCTXTXDIR_READ)
+            {
+                   int rc2 = vdFilterChainApplyRead(pDisk, pTmp->Req.Io.uOffsetXferOrig,
+                                                    pTmp->Req.Io.cbXferOrig, pTmp);
+                    if (RT_FAILURE(rc2))
+                        rcTmp = rc2;
+            }
+
             /* The given I/O context was processed, pass the return code to the caller. */
             if (   rcTmp == VINF_VD_ASYNC_IO_FINISHED
@@ -2209 +2228 @@
     IoCtx.Type.Root.pvUser2     = hEventComplete;
     rc = vdIoCtxProcessSync(&IoCtx, hEventComplete);
-    if (RT_SUCCESS(rc))
-        rc = vdFilterChainApplyRead(pDisk, uOffset, cbRead, &IoCtx);
-
     RTSemEventDestroy(hEventComplete);
     return rc;
@@ -2308 +2324 @@
     IoCtx.Type.Root.pvUser1     = pDisk;
     IoCtx.Type.Root.pvUser2     = hEventComplete;
-    /* Apply write filter chain here. */
-    rc = vdFilterChainApplyWrite(pDisk, uOffset, cbWrite, &IoCtx);
     if (RT_SUCCESS(rc))
         rc = vdIoCtxProcessSync(&IoCtx, hEventComplete);
@@ -2921 +2935 @@
     size_t cbThisWrite;
     size_t cbPreRead, cbPostRead;
+
+    /* Apply write filter chain here if it was not done already. */
+    if (!(pIoCtx->fFlags & VDIOCTX_FLAGS_WRITE_FILTER_APPLIED))
+    {
+        rc = vdFilterChainApplyWrite(pDisk, uOffset, cbWrite, pIoCtx);
+        if (RT_FAILURE(rc))
+            return rc;
+        pIoCtx->fFlags |= VDIOCTX_FLAGS_WRITE_FILTER_APPLIED;
+    }
 
     if (!(pIoCtx->fFlags & VDIOCTX_FLAGS_DONT_SET_MODIFIED_FLAG))
@@ -10051 +10074 @@
         {
             if (ASMAtomicCmpXchgBool(&pIoCtx->fComplete, true, false))
-            {
-                rc2 = vdFilterChainApplyRead(pDisk, pIoCtx->Req.Io.uOffsetXferOrig,
-                                             pIoCtx->Req.Io.cbXferOrig, pIoCtx);
-                if (RT_FAILURE(rc2))
-                    rc = rc2;
                 vdIoCtxFree(pDisk, pIoCtx);
-            }
             else
                 rc = VERR_VD_ASYNC_IO_IN_PROGRESS; /* Let the other handler complete the request. */
@@ -10122 +10139 @@
         {
             rc = VERR_NO_MEMORY;
-            break;
-        }
-
-        /* Apply write filter chain here. */
-        rc = vdFilterChainApplyWrite(pDisk, uOffset, cbWrite, pIoCtx);
-        if (RT_FAILURE(rc))
-        {
-            vdIoCtxFree(pDisk, pIoCtx);
             break;
         }