[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 26/32] qcow2: Fix qcow2_get_cluster_offset()
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] [PULL 26/32] qcow2: Fix qcow2_get_cluster_offset() |
Date: |
Fri, 8 Jul 2016 19:21:38 +0200 |
From: Max Reitz <address@hidden>
Recently, qcow2_get_cluster_offset() has been changed to work with bytes
instead of sectors. This invalidated some assertions and introduced a
possible integer multiplication overflow.
This could be reproduced using e.g.
$ qemu-img create -f qcow2 -o cluster_size=1M blub.qcow2 8G
Formatting 'foo.qcow2', fmt=qcow2 size=8589934592 encryption=off
cluster_size=1048576 lazy_refcounts=off refcount_bits=16
$ qemu-io -c map blub.qcow2
qemu-io: qemu/block/qcow2-cluster.c:504: qcow2_get_cluster_offset:
Assertion `bytes_needed <= INT_MAX' failed.
[1] 20775 abort (core dumped) qemu-io -c map foo.qcow2
This patch removes the now wrong assertion, adding comments and more
assertions to prove its correctness (and fixing the overflow which would
become apparent with the original assertion removed).
Signed-off-by: Max Reitz <address@hidden>
Message-id: address@hidden
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Max Reitz <address@hidden>
---
block/qcow2-cluster.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 00c16dc..f941835 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -483,8 +483,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t
offset,
unsigned int l2_index;
uint64_t l1_index, l2_offset, *l2_table;
int l1_bits, c;
- unsigned int offset_in_cluster, nb_clusters;
- uint64_t bytes_available, bytes_needed;
+ unsigned int offset_in_cluster;
+ uint64_t bytes_available, bytes_needed, nb_clusters;
int ret;
offset_in_cluster = offset_into_cluster(s, offset);
@@ -500,7 +500,6 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t
offset,
if (bytes_needed > bytes_available) {
bytes_needed = bytes_available;
}
- assert(bytes_needed <= INT_MAX);
*cluster_offset = 0;
@@ -537,8 +536,11 @@ int qcow2_get_cluster_offset(BlockDriverState *bs,
uint64_t offset,
l2_index = (offset >> s->cluster_bits) & (s->l2_size - 1);
*cluster_offset = be64_to_cpu(l2_table[l2_index]);
- /* nb_needed <= INT_MAX, thus nb_clusters <= INT_MAX, too */
nb_clusters = size_to_clusters(s, bytes_needed);
+ /* bytes_needed <= *bytes + offset_in_cluster, both of which are unsigned
+ * integers; the minimum cluster size is 512, so this assertion is always
+ * true */
+ assert(nb_clusters <= INT_MAX);
ret = qcow2_get_cluster_type(*cluster_offset);
switch (ret) {
@@ -585,13 +587,17 @@ int qcow2_get_cluster_offset(BlockDriverState *bs,
uint64_t offset,
qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
- bytes_available = (c * s->cluster_size);
+ bytes_available = (int64_t)c * s->cluster_size;
out:
if (bytes_available > bytes_needed) {
bytes_available = bytes_needed;
}
+ /* bytes_available <= bytes_needed <= *bytes + offset_in_cluster;
+ * subtracting offset_in_cluster will therefore definitely yield something
+ * not exceeding UINT_MAX */
+ assert(bytes_available - offset_in_cluster <= UINT_MAX);
*bytes = bytes_available - offset_in_cluster;
return ret;
--
1.8.3.1
- [Qemu-devel] [PULL 07/32] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup', (continued)
- [Qemu-devel] [PULL 07/32] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup', Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 17/32] block/qdev: Allow node name for drive properties, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 19/32] commit: Fix use of error handling policy, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 18/32] block/qdev: Allow configuring WCE with qdev properties, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 08/32] stream: Add 'job-id' parameter to 'block-stream', Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 10/32] qemu-img: Set the ID of the block job in img_commit(), Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 20/32] block/qdev: Allow configuring rerror/werror with qdev properties, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 25/32] qemu-io: Use correct range limitations, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 09/32] commit: Add 'job-id' parameter to 'block-commit', Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 23/32] qemu-img: Use strerror() for generic resize error, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 26/32] qcow2: Fix qcow2_get_cluster_offset(),
Kevin Wolf <=
- [Qemu-devel] [PULL 21/32] qemu-iotests: Test setting WCE with qdev, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 11/32] blockjob: Update description of the 'device' field in the QMP API, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 22/32] block: Remove BB options from blockdev-add, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 24/32] qcow2: Avoid making the L1 table too big, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 28/32] vmdk: fix metadata write regression, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 27/32] Improve block job rate limiting for small bandwidth values, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 16/32] coroutine: move entry argument to qemu_coroutine_create, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 29/32] blockdev: Fix regression with the default naming of throttling groups, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 30/32] qemu-iotests: Test naming of throttling groups, Kevin Wolf, 2016/07/08
- [Qemu-devel] [PULL 31/32] hmp: use snapshot name to determine whether a snapshot is 'fully available', Kevin Wolf, 2016/07/08