qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Kevin Wolf <kwolf@redhat.com>, Alberto Garcia <berto@igalia.com>
Subject: [Qemu-devel] [PULL 01/20] qcow2: Prevent allocating refcount blocks at offset 0
Date: Tue, 14 Nov 2017 18:23:58 +0100	[thread overview]
Message-ID: <20171114172417.7654-2-mreitz@redhat.com> (raw)
In-Reply-To: <20171114172417.7654-1-mreitz@redhat.com>

From: Alberto Garcia <berto@igalia.com>

Each entry in the qcow2 cache contains an offset field indicating the
location of the data in the qcow2 image. If the offset is 0 then it
means that the entry contains no data and is available to be used when
needed.

Because of that it is not possible to store in the cache the first
cluster of the qcow2 image (offset = 0). This is not a problem because
that cluster always contains the qcow2 header and we're not using this
cache for that.

However, if the qcow2 image is corrupted it can happen that we try to
allocate a new refcount block at offset 0, triggering this assertion
and crashing QEMU:

  qcow2_cache_entry_mark_dirty: Assertion `c->entries[i].offset != 0' failed

This patch adds an explicit check for this scenario and a new test
case.

This problem was originally reported here:

   https://bugs.launchpad.net/qemu/+bug/1728615

Reported-by: R.Nageswara Sastry <nasastry@in.ibm.com>
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 92a2fadd10d58b423f269c1d1a309af161cdc73f.1509718618.git.berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-refcount.c     |  7 +++++++
 tests/qemu-iotests/060     | 11 +++++++++++
 tests/qemu-iotests/060.out |  8 ++++++++
 3 files changed, 26 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index aa3fd6cf17..9059996c4b 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -367,6 +367,13 @@ static int alloc_refcount_block(BlockDriverState *bs,
         return new_block;
     }
 
+    /* If we're allocating the block at offset 0 then something is wrong */
+    if (new_block == 0) {
+        qcow2_signal_corruption(bs, true, -1, -1, "Preventing invalid "
+                                "allocation of refcount block at offset 0");
+        return -EIO;
+    }
+
 #ifdef DEBUG_ALLOC2
     fprintf(stderr, "qcow2: Allocate refcount block %d for %" PRIx64
         " at %" PRIx64 "\n",
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 8e95c450eb..dead26aeaf 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -242,6 +242,17 @@ poke_file "$TEST_IMG" "$(($l2_offset+8))" "\x80\x00\x00\x00\x00\x06\x2a\x00"
 # Should emit two error messages
 $QEMU_IO -c "discard 0 64k" -c "read 64k 64k" "$TEST_IMG" | _filter_qemu_io
 
+echo
+echo "=== Testing empty refcount table with valid L1 and L2 tables ==="
+echo
+_make_test_img 64M
+$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
+poke_file "$TEST_IMG" "$rt_offset"        "\x00\x00\x00\x00\x00\x00\x00\x00"
+# Since the first data cluster is already allocated this triggers an
+# allocation with an explicit offset (using qcow2_alloc_clusters_at())
+# causing a refcount block to be allocated at offset 0
+$QEMU_IO -c "write 0 128k" "$TEST_IMG" | _filter_qemu_io
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 5ca3af491f..872719009c 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -181,4 +181,12 @@ qcow2: Marking image as corrupt: Cluster allocation offset 0x62a00 unaligned (L2
 discard 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read failed: Input/output error
+
+=== Testing empty refcount table with valid L1 and L2 tables ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qcow2: Marking image as corrupt: Preventing invalid allocation of refcount block at offset 0; further corruption events will be suppressed
+write failed: Input/output error
 *** done
-- 
2.13.6

  reply	other threads:[~2017-11-14 17:24 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-14 17:23 [Qemu-devel] [PULL 00/20] Block patches for 2.11.0-rc1 Max Reitz
2017-11-14 17:23 ` Max Reitz [this message]
2017-11-14 17:23 ` [Qemu-devel] [PULL 02/20] qcow2: Prevent allocating L2 tables at offset 0 Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 03/20] qcow2: Prevent allocating compressed clusters " Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 04/20] qcow2: Don't open images with header.refcount_table_clusters == 0 Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 05/20] qcow2: Add iotest for an image with header.refcount_table_offset " Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 06/20] qcow2: Add iotest for an empty refcount table Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 07/20] qcow2: Assert that the crypto header does not overlap other metadata Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 08/20] iotests: Make 030 less flaky Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 09/20] iotests: Add missing 'blkdebug::' in 040 Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 10/20] iotests: Make 055 less flaky Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 11/20] iotests: Make 083 " Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 12/20] iotests: Make 136 " Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 13/20] iotests: Use new-style NBD connections Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 14/20] qcow2: Check that corrupted images can be repaired in iotest 060 Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 15/20] block/snapshot: dirty all dirty bitmaps on snapshot-switch Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 16/20] iotests: 077: Filter out 'resume' lines Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 17/20] block/vhdx.c: Don't blindly update the header Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 18/20] block/parallels: Do not update header or truncate image when INMIGRATE Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 19/20] block/parallels: add migration blocker Max Reitz
2017-11-14 17:24 ` [Qemu-devel] [PULL 20/20] qemu-iotests: update unsupported image formats in 194 Max Reitz
2017-11-14 17:28 ` [Qemu-devel] [PULL 00/20] Block patches for 2.11.0-rc1 Peter Maydell
2017-11-14 17:31   ` Max Reitz
2017-11-14 18:30 ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171114172417.7654-2-mreitz@redhat.com \
    --to=mreitz@redhat.com \
    --cc=berto@igalia.com \
    --cc=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).