Changeset 38518 in vbox

Timestamp:

Aug 25, 2011 7:46:41 AM (14 years ago)

Author:

vboxsync

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

73646

Message:

VHD: Clip read size before checking if the block is allocated. Fixes incorrect reads which cross block bounaries

File:

• trunk/src/VBox/Storage/VHD.cpp (modified) (1 diff)

trunk/src/VBox/Storage/VHD.cpp

@@ -1452 +1452 @@
 
         /*
+         * Clip read range to remain in this data block.
+         */
+        cbBuf = RT_MIN(cbBuf, (pImage->cbDataBlock - (cBATEntryIndex * VHD_SECTOR_SIZE)));
+
+        /*
          * If the block is not allocated the content of the entry is ~0
          */
         if (pImage->pBlockAllocationTable[cBlockAllocationTableEntry] == ~0U)
-        {
-            /* Return block size as read. */
-            *pcbActuallyRead = RT_MIN(cbBuf, pImage->cSectorsPerDataBlock * VHD_SECTOR_SIZE);
             rc = VERR_VD_BLOCK_FREE;
-            goto out;
-        }
-
-        uVhdOffset = ((uint64_t)pImage->pBlockAllocationTable[cBlockAllocationTableEntry] + pImage->cDataBlockBitmapSectors + cBATEntryIndex) * VHD_SECTOR_SIZE;
-        LogFlowFunc(("uVhdOffset=%llu cbBuf=%u\n", uVhdOffset, cbBuf));
-
-        /*
-         * Clip read range to remain in this data block.
-         */
-        cbBuf = RT_MIN(cbBuf, (pImage->cbDataBlock - (cBATEntryIndex * VHD_SECTOR_SIZE)));
-
-        /* Read in the block's bitmap. */
-        rc = vdIfIoIntFileReadSync(pImage->pIfIo, pImage->pStorage,
-                                   ((uint64_t)pImage->pBlockAllocationTable[cBlockAllocationTableEntry]) * VHD_SECTOR_SIZE,
-                                   pImage->pu8Bitmap, pImage->cbDataBlockBitmap,
-                                   NULL);
-        if (RT_SUCCESS(rc))
-        {
-            uint32_t cSectors = 0;
-
-            if (vhdBlockBitmapSectorContainsData(pImage, cBATEntryIndex))
+        {
+            uVhdOffset = ((uint64_t)pImage->pBlockAllocationTable[cBlockAllocationTableEntry] + pImage->cDataBlockBitmapSectors + cBATEntryIndex) * VHD_SECTOR_SIZE;
+            LogFlowFunc(("uVhdOffset=%llu cbBuf=%u\n", uVhdOffset, cbBuf));
+
+            /* Read in the block's bitmap. */
+            rc = vdIfIoIntFileReadSync(pImage->pIfIo, pImage->pStorage,
+                                       ((uint64_t)pImage->pBlockAllocationTable[cBlockAllocationTableEntry]) * VHD_SECTOR_SIZE,
+                                       pImage->pu8Bitmap, pImage->cbDataBlockBitmap,
+                                       NULL);
+            if (RT_SUCCESS(rc))
             {
-                cBATEntryIndex++;
-                cSectors = 1;
-
-                /*
-                 * The first sector being read is marked dirty, read as much as we
-                 * can from child. Note that only sectors that are marked dirty
-                 * must be read from child.
-                 */
-                while (   (cSectors < (cbBuf / VHD_SECTOR_SIZE))
-                       && vhdBlockBitmapSectorContainsData(pImage, cBATEntryIndex))
+                uint32_t cSectors = 0;
+
+                if (vhdBlockBitmapSectorContainsData(pImage, cBATEntryIndex))
                 {
                     cBATEntryIndex++;
-                    cSectors++;
+                    cSectors = 1;
+
+                    /*
+                     * The first sector being read is marked dirty, read as much as we
+                     * can from child. Note that only sectors that are marked dirty
+                     * must be read from child.
+                     */
+                    while (   (cSectors < (cbBuf / VHD_SECTOR_SIZE))
+                           && vhdBlockBitmapSectorContainsData(pImage, cBATEntryIndex))
+                    {
+                        cBATEntryIndex++;
+                        cSectors++;
+                    }
+
+                    cbBuf = cSectors * VHD_SECTOR_SIZE;
+
+                    LogFlowFunc(("uVhdOffset=%llu cbBuf=%u\n", uVhdOffset, cbBuf));
+                    rc = vdIfIoIntFileReadSync(pImage->pIfIo, pImage->pStorage,
+                                               uVhdOffset, pvBuf, cbBuf, NULL);
                 }
-
-                cbBuf = cSectors * VHD_SECTOR_SIZE;
-
-                LogFlowFunc(("uVhdOffset=%llu cbBuf=%u\n", uVhdOffset, cbBuf));
-                rc = vdIfIoIntFileReadSync(pImage->pIfIo, pImage->pStorage,
-                                           uVhdOffset, pvBuf, cbBuf, NULL);
+                else
+                {
+                    /*
+                     * The first sector being read is marked clean, so we should read from
+                     * our parent instead, but only as much as there are the following
+                     * clean sectors, because the block may still contain dirty sectors
+                     * further on. We just need to compute the number of clean sectors
+                     * and pass it to our caller along with the notification that they
+                     * should be read from the parent.
+                     */
+                    cBATEntryIndex++;
+                    cSectors = 1;
+
+                    while (   (cSectors < (cbBuf / VHD_SECTOR_SIZE))
+                           && !vhdBlockBitmapSectorContainsData(pImage, cBATEntryIndex))
+                    {
+                        cBATEntryIndex++;
+                        cSectors++;
+                    }
+
+                    cbBuf = cSectors * VHD_SECTOR_SIZE;
+                    LogFunc(("Sectors free: uVhdOffset=%llu cbBuf=%u\n", uVhdOffset, cbBuf));
+                    rc = VERR_VD_BLOCK_FREE;
+                }
             }
             else
-            {
-                /*
-                 * The first sector being read is marked clean, so we should read from
-                 * our parent instead, but only as much as there are the following
-                 * clean sectors, because the block may still contain dirty sectors
-                 * further on. We just need to compute the number of clean sectors
-                 * and pass it to our caller along with the notification that they
-                 * should be read from the parent.
-                 */
-                cBATEntryIndex++;
-                cSectors = 1;
-
-                while (   (cSectors < (cbBuf / VHD_SECTOR_SIZE))
-                       && !vhdBlockBitmapSectorContainsData(pImage, cBATEntryIndex))
-                {
-                    cBATEntryIndex++;
-                    cSectors++;
-                }
-
-                cbBuf = cSectors * VHD_SECTOR_SIZE;
-                LogFunc(("Sectors free: uVhdOffset=%llu cbBuf=%u\n", uVhdOffset, cbBuf));
-                rc = VERR_VD_BLOCK_FREE;
-            }
-        }
-        else
-            AssertMsgFailed(("Reading block bitmap failed rc=%Rrc\n", rc));
+                AssertMsgFailed(("Reading block bitmap failed rc=%Rrc\n", rc));
+        }
     }
     else