[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 7/7] qcow2: Fix corruption bug in qcow2_detect_metadata_preallocat
From: |
Kevin Wolf |
Subject: |
[PULL 7/7] qcow2: Fix corruption bug in qcow2_detect_metadata_preallocation() |
Date: |
Fri, 25 Oct 2019 15:46:11 +0200 |
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: address@hidden
Reported-by: Michael Weiser <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
Tested-by: Michael Weiser <address@hidden>
Reviewed-by: Michael Weiser <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
---
block/qcow2-refcount.c | 2 ++
block/qcow2.c | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index ef965d7895..0d64bf5a5e 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 8b05933565..0bc69e6996 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) {
--
2.20.1
- [PULL 0/7] Block layer patches, Kevin Wolf, 2019/10/25
- [PULL 2/7] iotests: Skip read-only cases in 118 when run as root, Kevin Wolf, 2019/10/25
- [PULL 4/7] block/backup: drop dead code from backup_job_create, Kevin Wolf, 2019/10/25
- [PULL 1/7] qapi: add support for blkreplay driver, Kevin Wolf, 2019/10/25
- [PULL 5/7] doc: Describe missing generic -blockdev options, Kevin Wolf, 2019/10/25
- [PULL 3/7] blockdev: Use error_report() in hmp_commit(), Kevin Wolf, 2019/10/25
- [PULL 7/7] qcow2: Fix corruption bug in qcow2_detect_metadata_preallocation(),
Kevin Wolf <=
- [PULL 6/7] coroutine: Add qemu_co_mutex_assert_locked(), Kevin Wolf, 2019/10/25
- Re: [PULL 0/7] Block layer patches, Peter Maydell, 2019/10/25