[Qemu-devel] [PULL 0/7] Block patches for 3.0-rc0

[Qemu-devel] [PULL 0/7] Block patches for 3.0-rc0

[Max Reitz]

The following changes since commit ec7eb2ae77cc207064e17ace048f7ec0c4b85d6f:

  translate-all: honour CF_NOCACHE in tb_gen_code (2018-07-09 16:14:36 +0100)

are available in the Git repository at:

  https://git.xanclic.moe/XanClic/qemu.git tags/pull-block-2018-07-09

for you to fetch changes up to 0e4e4318eaa56c831001bdf617094807ec6d451c:

  qcow2: add overlap check for bitmap directory (2018-07-09 19:43:24 +0200)

----------------------------------------------------------------
Block patches for 3.0-rc0:
- qcow2 metadata overlap protection for the persistent bitmap directory
- Various bug fixes

----------------------------------------------------------------
Fam Zheng (4):
  qcow2: Drop unused cluster_data
  file-posix: Fix fd_open check in raw_co_copy_range_to
  qcow2: Drop unreachable break
  raw: Drop superfluous semicolon

Max Reitz (2):
  vmdk: Fix possible segfault with non-VMDK backing
  iotests: Add VMDK backing file correlation test

Vladimir Sementsov-Ogievskiy (1):
  qcow2: add overlap check for bitmap directory

 qapi/block-core.json       |  21 +++---
 block/qcow2.h              |  45 +++++++------
 block/file-posix.c         |   2 +-
 block/qcow2-bitmap.c       |   7 +-
 block/qcow2-refcount.c     |  10 +++
 block/qcow2.c              |  25 +++----
 block/raw-format.c         |   2 +-
 block/vmdk.c               |   6 ++
 tests/qemu-iotests/225     | 132 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/225.out |  24 +++++++
 tests/qemu-iotests/group   |   1 +
 11 files changed, 231 insertions(+), 44 deletions(-)
 create mode 100755 tests/qemu-iotests/225
 create mode 100644 tests/qemu-iotests/225.out

-- 
2.17.1

[Qemu-devel] [PULL 1/7] qcow2: Drop unused cluster_data

[Max Reitz]

From: Fam Zheng <famz@redhat.com>

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 20180702025836.20957-2-famz@redhat.com
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 33b61b7480..d4ba0f781a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3345,7 +3345,6 @@ qcow2_co_copy_range_to(BlockDriverState *bs,
     int ret;
     unsigned int cur_bytes; /* number of sectors in current iteration */
     uint64_t cluster_offset;
-    uint8_t *cluster_data = NULL;
     QCowL2Meta *l2meta = NULL;
 
     assert(!bs->encrypted);
@@ -3404,7 +3403,6 @@ fail:
 
     qemu_co_mutex_unlock(&s->lock);
 
-    qemu_vfree(cluster_data);
     trace_qcow2_writev_done_req(qemu_coroutine_self(), ret);
 
     return ret;
-- 
2.17.1

[Qemu-devel] [PULL 2/7] file-posix: Fix fd_open check in raw_co_copy_range_to

[Max Reitz]

From: Fam Zheng <famz@redhat.com>

One of them is a typo. But update both to be more readable.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 20180702025836.20957-3-famz@redhat.com
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 98987b80f1..349f77a3af 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2611,7 +2611,7 @@ static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs,
     }
 
     src_s = src->bs->opaque;
-    if (fd_open(bs) < 0 || fd_open(bs) < 0) {
+    if (fd_open(src->bs) < 0 || fd_open(dst->bs) < 0) {
         return -EIO;
     }
     return paio_submit_co_full(bs, src_s->fd, src_offset, s->fd, dst_offset,
-- 
2.17.1

[Qemu-devel] [PULL 3/7] qcow2: Drop unreachable break

[Max Reitz]

From: Fam Zheng <famz@redhat.com>

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 20180702025836.20957-4-famz@redhat.com
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index d4ba0f781a..f327116e52 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3299,7 +3299,6 @@ qcow2_co_copy_range_from(BlockDriverState *bs,
         case QCOW2_CLUSTER_COMPRESSED:
             ret = -ENOTSUP;
             goto out;
-            break;
 
         case QCOW2_CLUSTER_NORMAL:
             child = bs->file;
-- 
2.17.1

[Qemu-devel] [PULL 4/7] raw: Drop superfluous semicolon

[Max Reitz]

From: Fam Zheng <famz@redhat.com>

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 20180702025836.20957-5-famz@redhat.com
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/raw-format.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/raw-format.c b/block/raw-format.c
index b78da564d4..8e648a5666 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -177,7 +177,7 @@ static inline int raw_adjust_offset(BlockDriverState *bs, uint64_t *offset,
         /* There's not enough space for the write, or the read request is
          * out-of-range. Don't read/write anything to prevent leaking out of
          * the size specified in options. */
-        return is_write ? -ENOSPC : -EINVAL;;
+        return is_write ? -ENOSPC : -EINVAL;
     }
 
     if (*offset > INT64_MAX - s->offset) {
-- 
2.17.1

[Qemu-devel] [PULL 5/7] vmdk: Fix possible segfault with non-VMDK backing

[Max Reitz]

VMDK performs a probing check in vmdk_co_create_opts() to prevent the
user from assigning non-VMDK files as a backing file, because it only
supports VMDK backing files.  However, with the @backing runtime option,
it is possible to assign arbitrary nodes as backing nodes, regardless of
what the image header says.  Therefore, VMDK may not just access backing
nodes assuming they are VMDK nodes -- which it does, because it needs to
compare the backing file's CID with the overlay's parentCID value, and
naturally the backing file only has a CID when it's a VMDK file.
Instead, it should report the CID of non-VMDK backing files not to match
the overlay because clearly a non-present CID does not match.

Without this change, vmdk_read_cid() reads from the backing file's
bs->file, which may be NULL (in which case we get a segfault).  Also, it
interprets bs->opaque as a BDRVVmdkState and then reads from the
.desc_offset field, which usually will just return some arbitrary value
which then results in either garbage to be read, or bdrv_pread() to
return an error, both of which result in a non-matching CID to be
reported.

(In a very unlikely case, we could read something that looks like a
VMDK descriptor, and then get a CID which might actually match.  But
that is highly unlikely, and the only result would be that VMDK accepts
the backing file which is not too bad (albeit unintentional).)

((And in theory, the seek to .desc_offset might leak data from another
block driver's opaque object.  But then again, the user should realize
very quickly that a non-VMDK backing file does not work (because the
read will very likely fail, due to the reasons given above), so this
should not be exploitable.))

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180702210721.4847-2-mreitz@redhat.com
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/vmdk.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index 84f8bbe480..a9d0084e36 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -333,6 +333,12 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
     if (!s->cid_checked && bs->backing) {
         BlockDriverState *p_bs = bs->backing->bs;
 
+        if (strcmp(p_bs->drv->format_name, "vmdk")) {
+            /* Backing file is not in vmdk format, so it does not have
+             * a CID, which makes the overlay's parent CID invalid */
+            return 0;
+        }
+
         if (vmdk_read_cid(p_bs, 0, &cur_pcid) != 0) {
             /* read failure: report as not valid */
             return 0;
-- 
2.17.1

[Qemu-devel] [PULL 6/7] iotests: Add VMDK backing file correlation test

[Max Reitz]

This new test verifies that VMDK backing file reads fail when the
backing file has a non-matching CID.  This includes non-VMDK backing
files.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180702210721.4847-3-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/225     | 132 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/225.out |  24 +++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 157 insertions(+)
 create mode 100755 tests/qemu-iotests/225
 create mode 100644 tests/qemu-iotests/225.out

diff --git a/tests/qemu-iotests/225 b/tests/qemu-iotests/225
new file mode 100755
index 0000000000..f2ee715685
--- /dev/null
+++ b/tests/qemu-iotests/225
@@ -0,0 +1,132 @@
+#!/bin/bash
+#
+# Test vmdk backing file correlation
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+here=$PWD
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+    rm -f "$TEST_IMG.not_base"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+# This tests vmdk-specific low-level functionality
+_supported_fmt vmdk
+_supported_proto file
+_supported_os Linux
+_unsupported_imgopts "subformat=monolithicFlat" \
+                     "subformat=twoGbMaxExtentFlat" \
+                     "subformat=twoGbMaxExtentSparse"
+
+TEST_IMG="$TEST_IMG.base" _make_test_img 1M
+TEST_IMG="$TEST_IMG.not_base" _make_test_img 1M
+_make_test_img -b "$TEST_IMG.base"
+
+make_opts()
+{
+    node_name=$1
+    filename=$2
+    backing=$3
+
+    if [ -z "$backing" ]; then
+        backing="null"
+    else
+        backing="'$backing'"
+    fi
+
+    echo "{ 'node-name': '$node_name',
+            'driver': 'vmdk',
+            'file': {
+                'driver': 'file',
+                'filename': '$filename'
+            },
+            'backing': $backing }"
+}
+
+overlay_opts=$(make_opts overlay "$TEST_IMG" backing)
+base_opts=$(make_opts backing "$TEST_IMG.base")
+not_base_opts=$(make_opts backing "$TEST_IMG.not_base")
+
+not_vmdk_opts="{ 'node-name': 'backing', 'driver': 'null-co' }"
+
+echo
+echo '=== Testing fitting VMDK backing image ==='
+echo
+
+qemu_comm_method=monitor \
+    _launch_qemu -blockdev "$base_opts" -blockdev "$overlay_opts"
+
+# Should not return an error
+_send_qemu_cmd $QEMU_HANDLE 'qemu-io overlay "read 0 512"' 'ops'
+
+_cleanup_qemu
+
+
+echo
+echo '=== Testing unrelated VMDK backing image ==='
+echo
+
+qemu_comm_method=monitor \
+    _launch_qemu -blockdev "$not_base_opts" -blockdev "$overlay_opts"
+
+# Should fail (gracefully)
+_send_qemu_cmd $QEMU_HANDLE 'qemu-io overlay "read 0 512"' 'failed'
+
+_cleanup_qemu
+
+
+echo
+echo '=== Testing non-VMDK backing image ==='
+echo
+
+# FIXME: This is the reason why we have to use two -blockdev
+# invocations.  You can only fully override the backing file options
+# if you either specify a node reference (as done here) or the new
+# options contain file.filename (which in this case they do not).
+# In other cases, file.filename will be set to whatever the image
+# header of the overlay contains (which we do not want).  I consider
+# this a FIXME because with -blockdev, you cannot specify "partial"
+# options, so setting file.filename but leaving the rest as specified
+# by the user does not make sense.
+qemu_comm_method=monitor \
+    _launch_qemu -blockdev "$not_vmdk_opts" -blockdev "$overlay_opts"
+
+# Should fail (gracefully)
+_send_qemu_cmd $QEMU_HANDLE 'qemu-io overlay "read 0 512"' 'failed'
+
+_cleanup_qemu
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/225.out b/tests/qemu-iotests/225.out
new file mode 100644
index 0000000000..4dc8ee282f
--- /dev/null
+++ b/tests/qemu-iotests/225.out
@@ -0,0 +1,24 @@
+QA output created by 225
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+Formatting 'TEST_DIR/t.IMGFMT.not_base', fmt=IMGFMT size=1048576
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.base
+
+=== Testing fitting VMDK backing image ===
+
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) qemu-io overlay "read 0 512"
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing unrelated VMDK backing image ===
+
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) qemu-io overlay "read 0 512"
+read failed: Invalid argument
+
+=== Testing non-VMDK backing image ===
+
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) qemu-io overlay "read 0 512"
+read failed: Invalid argument
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index af309ebba7..1c9f679821 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -222,3 +222,4 @@
 221 rw auto quick
 222 rw auto quick
 223 rw auto quick
+225 rw auto quick
-- 
2.17.1

[Qemu-devel] [PULL 7/7] qcow2: add overlap check for bitmap directory

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20180705151515.779173-1-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qapi/block-core.json   | 21 +++++++++++---------
 block/qcow2.h          | 45 ++++++++++++++++++++++--------------------
 block/qcow2-bitmap.c   |  7 ++++++-
 block/qcow2-refcount.c | 10 ++++++++++
 block/qcow2.c          | 22 +++++++++++++--------
 5 files changed, 66 insertions(+), 39 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 38b31250f9..13798b982d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2696,18 +2696,21 @@
 # @template: Specifies a template mode which can be adjusted using the other
 #            flags, defaults to 'cached'
 #
+# @bitmap-directory: since 3.0
+#
 # Since: 2.9
 ##
 { 'struct': 'Qcow2OverlapCheckFlags',
-  'data': { '*template':       'Qcow2OverlapCheckMode',
-            '*main-header':    'bool',
-            '*active-l1':      'bool',
-            '*active-l2':      'bool',
-            '*refcount-table': 'bool',
-            '*refcount-block': 'bool',
-            '*snapshot-table': 'bool',
-            '*inactive-l1':    'bool',
-            '*inactive-l2':    'bool' } }
+  'data': { '*template':         'Qcow2OverlapCheckMode',
+            '*main-header':      'bool',
+            '*active-l1':        'bool',
+            '*active-l2':        'bool',
+            '*refcount-table':   'bool',
+            '*refcount-block':   'bool',
+            '*snapshot-table':   'bool',
+            '*inactive-l1':      'bool',
+            '*inactive-l2':      'bool',
+            '*bitmap-directory': 'bool' } }
 
 ##
 # @Qcow2OverlapChecks:
diff --git a/block/qcow2.h b/block/qcow2.h
index d6aca687d6..81b844e936 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -94,6 +94,7 @@
 #define QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE "overlap-check.snapshot-table"
 #define QCOW2_OPT_OVERLAP_INACTIVE_L1 "overlap-check.inactive-l1"
 #define QCOW2_OPT_OVERLAP_INACTIVE_L2 "overlap-check.inactive-l2"
+#define QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY "overlap-check.bitmap-directory"
 #define QCOW2_OPT_CACHE_SIZE "cache-size"
 #define QCOW2_OPT_L2_CACHE_SIZE "l2-cache-size"
 #define QCOW2_OPT_L2_CACHE_ENTRY_SIZE "l2-cache-entry-size"
@@ -400,34 +401,36 @@ typedef enum QCow2ClusterType {
 } QCow2ClusterType;
 
 typedef enum QCow2MetadataOverlap {
-    QCOW2_OL_MAIN_HEADER_BITNR    = 0,
-    QCOW2_OL_ACTIVE_L1_BITNR      = 1,
-    QCOW2_OL_ACTIVE_L2_BITNR      = 2,
-    QCOW2_OL_REFCOUNT_TABLE_BITNR = 3,
-    QCOW2_OL_REFCOUNT_BLOCK_BITNR = 4,
-    QCOW2_OL_SNAPSHOT_TABLE_BITNR = 5,
-    QCOW2_OL_INACTIVE_L1_BITNR    = 6,
-    QCOW2_OL_INACTIVE_L2_BITNR    = 7,
-
-    QCOW2_OL_MAX_BITNR            = 8,
-
-    QCOW2_OL_NONE           = 0,
-    QCOW2_OL_MAIN_HEADER    = (1 << QCOW2_OL_MAIN_HEADER_BITNR),
-    QCOW2_OL_ACTIVE_L1      = (1 << QCOW2_OL_ACTIVE_L1_BITNR),
-    QCOW2_OL_ACTIVE_L2      = (1 << QCOW2_OL_ACTIVE_L2_BITNR),
-    QCOW2_OL_REFCOUNT_TABLE = (1 << QCOW2_OL_REFCOUNT_TABLE_BITNR),
-    QCOW2_OL_REFCOUNT_BLOCK = (1 << QCOW2_OL_REFCOUNT_BLOCK_BITNR),
-    QCOW2_OL_SNAPSHOT_TABLE = (1 << QCOW2_OL_SNAPSHOT_TABLE_BITNR),
-    QCOW2_OL_INACTIVE_L1    = (1 << QCOW2_OL_INACTIVE_L1_BITNR),
+    QCOW2_OL_MAIN_HEADER_BITNR      = 0,
+    QCOW2_OL_ACTIVE_L1_BITNR        = 1,
+    QCOW2_OL_ACTIVE_L2_BITNR        = 2,
+    QCOW2_OL_REFCOUNT_TABLE_BITNR   = 3,
+    QCOW2_OL_REFCOUNT_BLOCK_BITNR   = 4,
+    QCOW2_OL_SNAPSHOT_TABLE_BITNR   = 5,
+    QCOW2_OL_INACTIVE_L1_BITNR      = 6,
+    QCOW2_OL_INACTIVE_L2_BITNR      = 7,
+    QCOW2_OL_BITMAP_DIRECTORY_BITNR = 8,
+
+    QCOW2_OL_MAX_BITNR              = 9,
+
+    QCOW2_OL_NONE             = 0,
+    QCOW2_OL_MAIN_HEADER      = (1 << QCOW2_OL_MAIN_HEADER_BITNR),
+    QCOW2_OL_ACTIVE_L1        = (1 << QCOW2_OL_ACTIVE_L1_BITNR),
+    QCOW2_OL_ACTIVE_L2        = (1 << QCOW2_OL_ACTIVE_L2_BITNR),
+    QCOW2_OL_REFCOUNT_TABLE   = (1 << QCOW2_OL_REFCOUNT_TABLE_BITNR),
+    QCOW2_OL_REFCOUNT_BLOCK   = (1 << QCOW2_OL_REFCOUNT_BLOCK_BITNR),
+    QCOW2_OL_SNAPSHOT_TABLE   = (1 << QCOW2_OL_SNAPSHOT_TABLE_BITNR),
+    QCOW2_OL_INACTIVE_L1      = (1 << QCOW2_OL_INACTIVE_L1_BITNR),
     /* NOTE: Checking overlaps with inactive L2 tables will result in bdrv
      * reads. */
-    QCOW2_OL_INACTIVE_L2    = (1 << QCOW2_OL_INACTIVE_L2_BITNR),
+    QCOW2_OL_INACTIVE_L2      = (1 << QCOW2_OL_INACTIVE_L2_BITNR),
+    QCOW2_OL_BITMAP_DIRECTORY = (1 << QCOW2_OL_BITMAP_DIRECTORY_BITNR),
 } QCow2MetadataOverlap;
 
 /* Perform all overlap checks which can be done in constant time */
 #define QCOW2_OL_CONSTANT \
     (QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_REFCOUNT_TABLE | \
-     QCOW2_OL_SNAPSHOT_TABLE)
+     QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_BITMAP_DIRECTORY)
 
 /* Perform all overlap checks which don't require disk access */
 #define QCOW2_OL_CACHED \
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 69485aa1de..ba978ad2aa 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -775,7 +775,12 @@ static int bitmap_list_store(BlockDriverState *bs, Qcow2BitmapList *bm_list,
         }
     }
 
-    ret = qcow2_pre_write_overlap_check(bs, 0, dir_offset, dir_size);
+    /* Actually, even in in-place case ignoring QCOW2_OL_BITMAP_DIRECTORY is not
+     * necessary, because we drop QCOW2_AUTOCLEAR_BITMAPS when updating bitmap
+     * directory in-place (actually, turn-off the extension), which is checked
+     * in qcow2_check_metadata_overlap() */
+    ret = qcow2_pre_write_overlap_check(
+            bs, in_place ? QCOW2_OL_BITMAP_DIRECTORY : 0, dir_offset, dir_size);
     if (ret < 0) {
         goto fail;
     }
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 18c729aa27..1b9ecb1ca0 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -2705,6 +2705,16 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
         }
     }
 
+    if ((chk & QCOW2_OL_BITMAP_DIRECTORY) &&
+        (s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS))
+    {
+        if (overlaps_with(s->bitmap_directory_offset,
+                          s->bitmap_directory_size))
+        {
+            return QCOW2_OL_BITMAP_DIRECTORY;
+        }
+    }
+
     return 0;
 }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index f327116e52..5d668fc617 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -679,6 +679,11 @@ static QemuOptsList qcow2_runtime_opts = {
             .type = QEMU_OPT_BOOL,
             .help = "Check for unintended writes into an inactive L2 table",
         },
+        {
+            .name = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
+            .type = QEMU_OPT_BOOL,
+            .help = "Check for unintended writes into the bitmap directory",
+        },
         {
             .name = QCOW2_OPT_CACHE_SIZE,
             .type = QEMU_OPT_SIZE,
@@ -712,14 +717,15 @@ static QemuOptsList qcow2_runtime_opts = {
 };
 
 static const char *overlap_bool_option_names[QCOW2_OL_MAX_BITNR] = {
-    [QCOW2_OL_MAIN_HEADER_BITNR]    = QCOW2_OPT_OVERLAP_MAIN_HEADER,
-    [QCOW2_OL_ACTIVE_L1_BITNR]      = QCOW2_OPT_OVERLAP_ACTIVE_L1,
-    [QCOW2_OL_ACTIVE_L2_BITNR]      = QCOW2_OPT_OVERLAP_ACTIVE_L2,
-    [QCOW2_OL_REFCOUNT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_TABLE,
-    [QCOW2_OL_REFCOUNT_BLOCK_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK,
-    [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE,
-    [QCOW2_OL_INACTIVE_L1_BITNR]    = QCOW2_OPT_OVERLAP_INACTIVE_L1,
-    [QCOW2_OL_INACTIVE_L2_BITNR]    = QCOW2_OPT_OVERLAP_INACTIVE_L2,
+    [QCOW2_OL_MAIN_HEADER_BITNR]      = QCOW2_OPT_OVERLAP_MAIN_HEADER,
+    [QCOW2_OL_ACTIVE_L1_BITNR]        = QCOW2_OPT_OVERLAP_ACTIVE_L1,
+    [QCOW2_OL_ACTIVE_L2_BITNR]        = QCOW2_OPT_OVERLAP_ACTIVE_L2,
+    [QCOW2_OL_REFCOUNT_TABLE_BITNR]   = QCOW2_OPT_OVERLAP_REFCOUNT_TABLE,
+    [QCOW2_OL_REFCOUNT_BLOCK_BITNR]   = QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK,
+    [QCOW2_OL_SNAPSHOT_TABLE_BITNR]   = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE,
+    [QCOW2_OL_INACTIVE_L1_BITNR]      = QCOW2_OPT_OVERLAP_INACTIVE_L1,
+    [QCOW2_OL_INACTIVE_L2_BITNR]      = QCOW2_OPT_OVERLAP_INACTIVE_L2,
+    [QCOW2_OL_BITMAP_DIRECTORY_BITNR] = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
 };
 
 static void cache_clean_timer_cb(void *opaque)
-- 
2.17.1

Re: [Qemu-devel] [PULL 0/7] Block patches for 3.0-rc0

[Peter Maydell]

On 9 July 2018 at 19:58, Max Reitz <mreitz@redhat.com> wrote:
> The following changes since commit ec7eb2ae77cc207064e17ace048f7ec0c4b85d6f:
>
>   translate-all: honour CF_NOCACHE in tb_gen_code (2018-07-09 16:14:36 +0100)
>
> are available in the Git repository at:
>
>   https://git.xanclic.moe/XanClic/qemu.git tags/pull-block-2018-07-09
>
> for you to fetch changes up to 0e4e4318eaa56c831001bdf617094807ec6d451c:
>
>   qcow2: add overlap check for bitmap directory (2018-07-09 19:43:24 +0200)
>
> ----------------------------------------------------------------
> Block patches for 3.0-rc0:
> - qcow2 metadata overlap protection for the persistent bitmap directory
> - Various bug fixes
>
> ----------------------------------------------------------------
> Fam Zheng (4):
>   qcow2: Drop unused cluster_data
>   file-posix: Fix fd_open check in raw_co_copy_range_to
>   qcow2: Drop unreachable break
>   raw: Drop superfluous semicolon
>
> Max Reitz (2):
>   vmdk: Fix possible segfault with non-VMDK backing
>   iotests: Add VMDK backing file correlation test
>
> Vladimir Sementsov-Ogievskiy (1):
>   qcow2: add overlap check for bitmap directory

Applied, thanks.

-- PMM