All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alberto Garcia <berto@igalia.com>
To: qemu-devel@nongnu.org
Cc: Alberto Garcia <berto@igalia.com>,
	qemu-block@nongnu.org, Eric Blake <eblake@redhat.com>,
	Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>
Subject: [Qemu-devel] [PATCH 3/7] qcow2: Check L1 table parameters in qcow2_expand_zero_clusters()
Date: Thu,  1 Mar 2018 18:27:09 +0200	[thread overview]
Message-ID: <d58bbc353563bb9c83ffe90faaa36fadf683e332.1519921268.git.berto@igalia.com> (raw)
In-Reply-To: <cover.1519921268.git.berto@igalia.com>
In-Reply-To: <cover.1519921268.git.berto@igalia.com>

This function iterates over all snapshots of a qcow2 file in order to
expand all zero clusters, but it does not validate the snapshots' L1
tables first.

We now have a function to take care of this, so let's use it.

We can also take the opportunity to replace the sector-based
bdrv_read() with bdrv_pread().

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2-cluster.c      | 20 +++++++++++++-------
 tests/qemu-iotests/080     |  2 ++
 tests/qemu-iotests/080.out |  2 ++
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index e406b0f3b9..40167ac09c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2092,11 +2092,18 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs,
     }
 
     for (i = 0; i < s->nb_snapshots; i++) {
-        int l1_sectors = DIV_ROUND_UP(s->snapshots[i].l1_size *
-                                      sizeof(uint64_t), BDRV_SECTOR_SIZE);
+        int l1_size2;
+        uint64_t *new_l1_table;
 
-        uint64_t *new_l1_table =
-            g_try_realloc(l1_table, l1_sectors * BDRV_SECTOR_SIZE);
+        ret = qcow2_validate_table(bs, s->snapshots[i].l1_table_offset,
+                                   s->snapshots[i].l1_size, sizeof(uint64_t),
+                                   QCOW_MAX_L1_SIZE, "", NULL);
+        if (ret < 0) {
+            return ret;
+        }
+
+        l1_size2 = s->snapshots[i].l1_size * sizeof(uint64_t);
+        new_l1_table = g_try_realloc(l1_table, l1_size2);
 
         if (!new_l1_table) {
             ret = -ENOMEM;
@@ -2105,9 +2112,8 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs,
 
         l1_table = new_l1_table;
 
-        ret = bdrv_read(bs->file,
-                        s->snapshots[i].l1_table_offset / BDRV_SECTOR_SIZE,
-                        (void *)l1_table, l1_sectors);
+        ret = bdrv_pread(bs->file, s->snapshots[i].l1_table_offset,
+                         l1_table, l1_size2);
         if (ret < 0) {
             goto fail;
         }
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 6a10e7defa..5622604f83 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -177,6 +177,7 @@ _make_test_img 64M
 { $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_testdir
 poke_file "$TEST_IMG" "$offset_snap1_l1_offset" "\x00\x00\x00\x00\x00\x40\x02\x00"
 { $QEMU_IMG convert -s test $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_testdir
+{ $QEMU_IMG amend -o compat=0.10 $TEST_IMG; } 2>&1 | _filter_testdir
 
 echo
 echo "== Invalid snapshot L1 table size =="
@@ -185,6 +186,7 @@ _make_test_img 64M
 { $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_testdir
 poke_file "$TEST_IMG" "$offset_snap1_l1_size" "\x10\x00\x00\x00"
 { $QEMU_IMG convert -s test $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_testdir
+{ $QEMU_IMG amend -o compat=0.10 $TEST_IMG; } 2>&1 | _filter_testdir
 
 # success, all done
 echo "*** done"
diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
index f0d9038d55..5d9030ab93 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -64,10 +64,12 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 qemu-img: Failed to load snapshot: Snapshot L1 table offset invalid
+qemu-img: Error while amending options: Invalid argument
 
 == Invalid snapshot L1 table size ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 qemu-img: Failed to load snapshot: Snapshot L1 table too large
+qemu-img: Error while amending options: File too large
 *** done
-- 
2.11.0

  parent reply	other threads:[~2018-03-01 16:28 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-01 16:27 [Qemu-devel] [PATCH 0/7] Add checks for corruption in the snapshot table Alberto Garcia
2018-03-01 16:27 ` [Qemu-devel] [PATCH 1/7] qcow2: Generalize validate_table_offset() into qcow2_validate_table() Alberto Garcia
2018-03-01 19:21   ` Eric Blake
2018-03-01 16:27 ` [Qemu-devel] [PATCH 2/7] qcow2: Check L1 table offset in qcow2_snapshot_load_tmp() Alberto Garcia
2018-03-01 23:32   ` Eric Blake
2018-03-01 16:27 ` Alberto Garcia [this message]
2018-03-01 23:39   ` [Qemu-devel] [PATCH 3/7] qcow2: Check L1 table parameters in qcow2_expand_zero_clusters() Eric Blake
2018-03-06 14:54   ` Kevin Wolf
2018-03-06 15:01     ` Alberto Garcia
2018-03-06 15:11       ` Kevin Wolf
2018-03-06 15:16         ` Alberto Garcia
2018-03-01 16:27 ` [Qemu-devel] [PATCH 4/7] qcow2: Check snapshot L1 tables in qcow2_check_metadata_overlap() Alberto Garcia
2018-03-02  1:20   ` Eric Blake
2018-03-01 16:27 ` [Qemu-devel] [PATCH 5/7] qcow2: Check snapshot L1 table in qcow2_snapshot_goto() Alberto Garcia
2018-03-02  1:35   ` Eric Blake
2018-03-01 16:27 ` [Qemu-devel] [PATCH 6/7] qcow2: Check snapshot L1 table in qcow2_snapshot_delete() Alberto Garcia
2018-03-02  1:36   ` Eric Blake
2018-03-01 16:27 ` [Qemu-devel] [PATCH 7/7] qcow2: Make qemu-img check detect corrupted L1 tables in snapshots Alberto Garcia
2018-03-02  1:37   ` Eric Blake
2018-03-06 14:06 ` [Qemu-devel] [PATCH 0/7] Add checks for corruption in the snapshot table Kevin Wolf
2018-03-06 14:18   ` Alberto Garcia

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=d58bbc353563bb9c83ffe90faaa36fadf683e332.1519921268.git.berto@igalia.com \
    --to=berto@igalia.com \
    --cc=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.