[Qemu-devel] [PATCH] block/vmdk.c: Fixed major bug in VMDK WRITE and READ handling - FIXES DATA CORRUPTION

[Qemu-devel] [PATCH] block/vmdk.c: Fixed major bug in VMDK WRITE and READ handling - FIXES DATA CORRUPTION

[Gerhard Wiesinger]

Fixed a MAJOR BUG in VMDK files on file boundaries on reads
and ALSO ON WRITES WHICH MIGHT CORRUPT THE IMAGE AND DATA!!!!!!

Triggered for example with the following VMDK file (partly listed):
# Extent description
RW 4193792 FLAT "XP-W1-f001.vmdk" 0
RW 2097664 FLAT "XP-W1-f002.vmdk" 0
RW 4193792 FLAT "XP-W1-f003.vmdk" 0
RW 512 FLAT "XP-W1-f004.vmdk" 0
RW 4193792 FLAT "XP-W1-f005.vmdk" 0
RW 2097664 FLAT "XP-W1-f006.vmdk" 0
RW 4193792 FLAT "XP-W1-f007.vmdk" 0
RW 512 FLAT "XP-W1-f008.vmdk" 0

Patch includes:
1.) Patch fixes wrong calculation on extent boundaries. Especially it fixes the relativeness of the sector number to the current extent.

Verfied correctness with:
1.) Converted either with Virtualbox to VDI and then with qemu-img and then with qemu-img only
VBoxManage clonehd --format vdi /VM/XP-W/new/XP-W1.vmdk ~/.VirtualBox/Harddisks/XP-W1-new-test.vdi
./qemu-img convert -O raw ~/.VirtualBox/Harddisks/XP-W1-new-test.vdi /root/QEMU/VM-XP-W1/XP-W1-via-VBOX.img
md5sum /root/QEMU/VM-XP-W/XP-W1-direct.img
md5sum /root/QEMU/VM-XP-W/XP-W1-via-VBOX.img
=> same MD5 hash
2.) Verified debug log files
3.) Run Windows XP successfully
4.) chkdsk run successfully without any errors

Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com>
---
  block/vmdk.c | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 1a80e5a..51398c0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1092,6 +1092,7 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
      BDRVVmdkState *s = bs->opaque;
      int ret;
      uint64_t n, index_in_cluster;
+    uint64_t extent_begin_sector, extent_relative_sector_num;
      VmdkExtent *extent = NULL;
      uint64_t cluster_offset;

@@ -1103,7 +1104,9 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
          ret = get_cluster_offset(
                              bs, extent, NULL,
                              sector_num << 9, 0, &cluster_offset);
-        index_in_cluster = sector_num % extent->cluster_sectors;
+        extent_begin_sector = extent->end_sector - extent->sectors;
+        extent_relative_sector_num = sector_num - extent_begin_sector;
+        index_in_cluster = extent_relative_sector_num % extent->cluster_sectors;
          n = extent->cluster_sectors - index_in_cluster;
          if (n > nb_sectors) {
              n = nb_sectors;
@@ -1154,6 +1157,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
      VmdkExtent *extent = NULL;
      int n, ret;
      int64_t index_in_cluster;
+    uint64_t extent_begin_sector, extent_relative_sector_num;
      uint64_t cluster_offset;
      VmdkMetaData m_data;

@@ -1196,7 +1200,9 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
          if (ret) {
              return -EINVAL;
          }
-        index_in_cluster = sector_num % extent->cluster_sectors;
+        extent_begin_sector = extent->end_sector - extent->sectors;
+        extent_relative_sector_num = sector_num - extent_begin_sector;
+        index_in_cluster = extent_relative_sector_num % extent->cluster_sectors;
          n = extent->cluster_sectors - index_in_cluster;
          if (n > nb_sectors) {
              n = nb_sectors;
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH] block/vmdk.c: Fixed major bug in VMDK WRITE and READ handling - FIXES DATA CORRUPTION

[Kevin Wolf]

Am 10.11.2012 09:22, schrieb Gerhard Wiesinger:
> Fixed a MAJOR BUG in VMDK files on file boundaries on reads
> and ALSO ON WRITES WHICH MIGHT CORRUPT THE IMAGE AND DATA!!!!!!
> 
> Triggered for example with the following VMDK file (partly listed):
> # Extent description
> RW 4193792 FLAT "XP-W1-f001.vmdk" 0
> RW 2097664 FLAT "XP-W1-f002.vmdk" 0
> RW 4193792 FLAT "XP-W1-f003.vmdk" 0
> RW 512 FLAT "XP-W1-f004.vmdk" 0
> RW 4193792 FLAT "XP-W1-f005.vmdk" 0
> RW 2097664 FLAT "XP-W1-f006.vmdk" 0
> RW 4193792 FLAT "XP-W1-f007.vmdk" 0
> RW 512 FLAT "XP-W1-f008.vmdk" 0
> 
> Patch includes:
> 1.) Patch fixes wrong calculation on extent boundaries. Especially it fixes the relativeness of the sector number to the current extent.
> 
> Verfied correctness with:
> 1.) Converted either with Virtualbox to VDI and then with qemu-img and then with qemu-img only
> VBoxManage clonehd --format vdi /VM/XP-W/new/XP-W1.vmdk ~/.VirtualBox/Harddisks/XP-W1-new-test.vdi
> ./qemu-img convert -O raw ~/.VirtualBox/Harddisks/XP-W1-new-test.vdi /root/QEMU/VM-XP-W1/XP-W1-via-VBOX.img
> md5sum /root/QEMU/VM-XP-W/XP-W1-direct.img
> md5sum /root/QEMU/VM-XP-W/XP-W1-via-VBOX.img
> => same MD5 hash
> 2.) Verified debug log files
> 3.) Run Windows XP successfully
> 4.) chkdsk run successfully without any errors
> 
> Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com>

Thanks, applied to the block branch. Something corrupted your patch
(indentation of many unchanged lines differed by one space); I fixed it
for this patch, but please check what caused this so that your next
patch can apply without changes.

It would also be a great idea to add a (VMDK only) test case to
qemu-iotests that catches this kind of bugs.

Kevin