aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKevin Wolf <kwolf@redhat.com>2019-10-24 16:26:58 +0200
committerKevin Wolf <kwolf@redhat.com>2019-10-25 15:18:55 +0200
commit5e9785505210e2477e590e61b1ab100d0ec22b01 (patch)
treeb64a0291d957cc4e7a485e68b9094394808aa142
parent944f3d5dd216fcd8cb007eddd4f82dced0a15b3d (diff)
downloadqemu-5e9785505210e2477e590e61b1ab100d0ec22b01.zip
qemu-5e9785505210e2477e590e61b1ab100d0ec22b01.tar.gz
qemu-5e9785505210e2477e590e61b1ab100d0ec22b01.tar.bz2
qcow2: Fix corruption bug in qcow2_detect_metadata_preallocation()
qcow2_detect_metadata_preallocation() calls qcow2_get_refcount() which requires s->lock to be taken to protect its accesses to the refcount table and refcount blocks. However, nothing in this code path actually took the lock. This could cause the same cache entry to be used by two requests at the same time, for different tables at different offsets, resulting in image corruption. As it would be preferable to base the detection on consistent data (even though it's just heuristics), let's take the lock not only around the qcow2_get_refcount() calls, but around the whole function. This patch takes the lock in qcow2_co_block_status() earlier and asserts in qcow2_detect_metadata_preallocation() that we hold the lock. Fixes: 69f47505ee66afaa513305de0c1895a224e52c45 Cc: qemu-stable@nongnu.org Reported-by: Michael Weiser <michael.weiser@gmx.de> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Tested-by: Michael Weiser <michael.weiser@gmx.de> Reviewed-by: Michael Weiser <michael.weiser@gmx.de> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
-rw-r--r--block/qcow2-refcount.c2
-rw-r--r--block/qcow2.c3
2 files changed, 4 insertions, 1 deletions
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index ef965d7..0d64bf5 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -3455,6 +3455,8 @@ int qcow2_detect_metadata_preallocation(BlockDriverState *bs)
int64_t i, end_cluster, cluster_count = 0, threshold;
int64_t file_length, real_allocation, real_clusters;
+ qemu_co_mutex_assert_locked(&s->lock);
+
file_length = bdrv_getlength(bs->file->bs);
if (file_length < 0) {
return file_length;
diff --git a/block/qcow2.c b/block/qcow2.c
index 8b05933..0bc69e6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1916,6 +1916,8 @@ static int coroutine_fn qcow2_co_block_status(BlockDriverState *bs,
unsigned int bytes;
int status = 0;
+ qemu_co_mutex_lock(&s->lock);
+
if (!s->metadata_preallocation_checked) {
ret = qcow2_detect_metadata_preallocation(bs);
s->metadata_preallocation = (ret == 1);
@@ -1923,7 +1925,6 @@ static int coroutine_fn qcow2_co_block_status(BlockDriverState *bs,
}
bytes = MIN(INT_MAX, count);
- qemu_co_mutex_lock(&s->lock);
ret = qcow2_get_cluster_offset(bs, offset, &bytes, &cluster_offset);
qemu_co_mutex_unlock(&s->lock);
if (ret < 0) {