Patchwork QCOW2: bug fix - read base image beyond its size

login
register
mail settings
Submitter Chunqiang Tang
Date Feb. 3, 2011, 3:12 p.m.
Message ID <1296745969-28287-1-git-send-email-ctang@us.ibm.com>
Download mbox | patch
Permalink /patch/81673/
State New
Headers show

Comments

Chunqiang Tang - Feb. 3, 2011, 3:12 p.m.
This patch fixes the following bug in QCOW2. For a QCOW2 image that is larger
than its base image, when handling a read request straddling over the end of the
base image, the QCOW2 driver attempts to read beyond the end of the base image
and the request would fail.

This bug was found by Fast Virtual Disk (FVD)'s fully automated testing tool.
The following test triggered the bug.

dd if=/dev/zero of=/var/ramdisk/truth.raw count=0 bs=1 seek=1098561536
dd if=/dev/zero of=/var/ramdisk/zero-500M.raw count=0 bs=1 seek=593099264
./qemu-img create -f qcow2 -ocluster_size=65536,backing_fmt=blksim -b /var/ramdisk/zero-500M.raw /var/ramdisk/test.qcow2 1098561536
./qemu-io --auto --seed=30477694 --truth=/var/ramdisk/truth.raw --format=qcow2 --test=blksim:/var/ramdisk/test.qcow2 --verify_write=true --compare_before=false --compare_after=true --round=100000 --parallel=100 --io_size=10485760 --fail_prob=0 --cancel_prob=0 --instant_qemubh=true

Signed-off-by: Chunqiang Tang <ctang@us.ibm.com>
---
 block/qcow2.c |    5 ++---
 cutils.c      |   31 +++++++++++++++++++++++++++++++
 qemu-common.h |    2 ++
 3 files changed, 35 insertions(+), 3 deletions(-)
Kevin Wolf - Feb. 7, 2011, 1:48 p.m.
Am 03.02.2011 16:12, schrieb Chunqiang Tang:
> This patch fixes the following bug in QCOW2. For a QCOW2 image that is larger
> than its base image, when handling a read request straddling over the end of the
> base image, the QCOW2 driver attempts to read beyond the end of the base image
> and the request would fail.
> 
> This bug was found by Fast Virtual Disk (FVD)'s fully automated testing tool.
> The following test triggered the bug.
> 
> dd if=/dev/zero of=/var/ramdisk/truth.raw count=0 bs=1 seek=1098561536
> dd if=/dev/zero of=/var/ramdisk/zero-500M.raw count=0 bs=1 seek=593099264
> ./qemu-img create -f qcow2 -ocluster_size=65536,backing_fmt=blksim -b /var/ramdisk/zero-500M.raw /var/ramdisk/test.qcow2 1098561536
> ./qemu-io --auto --seed=30477694 --truth=/var/ramdisk/truth.raw --format=qcow2 --test=blksim:/var/ramdisk/test.qcow2 --verify_write=true --compare_before=false --compare_after=true --round=100000 --parallel=100 --io_size=10485760 --fail_prob=0 --cancel_prob=0 --instant_qemubh=true
> 
> Signed-off-by: Chunqiang Tang <ctang@us.ibm.com>

Thanks, fixed up the coding style (whitespace) and applied to the block
branch.

However, I wonder if we can't share some code between
qemu_iovec_memset_skip and qemu_iovec_copy. I think a cleanup patch
would be possible there.

Kevin

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index dbe4fdd..8c906d1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -355,7 +355,7 @@  int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov,
     else
         n1 = bs->total_sectors - sector_num;
 
-    qemu_iovec_memset(qiov, 0, 512 * (nb_sectors - n1));
+    qemu_iovec_memset_skip(qiov, 0, 512 * (nb_sectors - n1), 512 * n1);
 
     return n1;
 }
@@ -478,8 +478,7 @@  static void qcow2_aio_read_cb(void *opaque, int ret)
             if (n1 > 0) {
                 BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
                 acb->hd_aiocb = bdrv_aio_readv(bs->backing_hd, acb->sector_num,
-                                    &acb->hd_qiov, acb->cur_nr_sectors,
-				    qcow2_aio_read_cb, acb);
+                                    &acb->hd_qiov, n1, qcow2_aio_read_cb, acb);
                 if (acb->hd_aiocb == NULL)
                     goto done;
             } else {
diff --git a/cutils.c b/cutils.c
index 8d562b2..e215210 100644
--- a/cutils.c
+++ b/cutils.c
@@ -267,6 +267,37 @@  void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count)
     }
 }
 
+void qemu_iovec_memset_skip(QEMUIOVector *qiov, int c, size_t count, 
+                            size_t skip)
+{
+    int i;
+    size_t done;
+    void *iov_base;
+    uint64_t iov_len;
+
+    done = 0;
+    for (i = 0; (i < qiov->niov) && (done != count); i++) {
+        if (skip >= qiov->iov[i].iov_len) {
+            /* Skip the whole iov */
+            skip -= qiov->iov[i].iov_len;
+            continue;
+        } else {
+            /* Skip only part (or nothing) of the iov */
+            iov_base = (uint8_t*) qiov->iov[i].iov_base + skip;
+            iov_len = qiov->iov[i].iov_len - skip;
+            skip = 0;
+        }
+
+        if (done + iov_len > count) {
+            memset (iov_base, c, count - done);
+            break;
+        } else {
+            memset (iov_base, c, iov_len);
+        }
+        done += iov_len;
+    }
+}
+
 #ifndef _WIN32
 /* Sets a specific flag */
 int fcntl_setfl(int fd, int flag)
diff --git a/qemu-common.h b/qemu-common.h
index c7ff280..7951109 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -322,6 +322,8 @@  void qemu_iovec_reset(QEMUIOVector *qiov);
 void qemu_iovec_to_buffer(QEMUIOVector *qiov, void *buf);
 void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void *buf, size_t count);
 void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count);
+void qemu_iovec_memset_skip(QEMUIOVector *qiov, int c, size_t count, 
+                            size_t skip);
 
 struct Monitor;
 typedef struct Monitor Monitor;