[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL v2 26/34] qcow2: Fix qcow2_get_cluster_offset()
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] [PULL v2 26/34] qcow2: Fix qcow2_get_cluster_offset() |
Date: |
Wed, 13 Jul 2016 14:50:33 +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 v2 12/34] osdep: Introduce qemu_dup, (continued)
- [Qemu-devel] [PULL v2 12/34] osdep: Introduce qemu_dup, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 15/34] test-coroutine: prepare for the next patch, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 09/34] commit: Add 'job-id' parameter to 'block-commit', Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 17/34] block/qdev: Allow node name for drive properties, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 19/34] commit: Fix use of error handling policy, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 14/34] coroutine: use QSIMPLEQ instead of QTAILQ, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 18/34] block/qdev: Allow configuring WCE with qdev properties, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 20/34] block/qdev: Allow configuring rerror/werror with qdev properties, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 16/34] coroutine: move entry argument to qemu_coroutine_create, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 25/34] qemu-io: Use correct range limitations, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 26/34] qcow2: Fix qcow2_get_cluster_offset(),
Kevin Wolf <=
- [Qemu-devel] [PULL v2 24/34] qcow2: Avoid making the L1 table too big, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 28/34] vmdk: fix metadata write regression, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 22/34] block: Remove BB options from blockdev-add, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 29/34] blockdev: Fix regression with the default naming of throttling groups, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 31/34] hmp: use snapshot name to determine whether a snapshot is 'fully available', Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 30/34] qemu-iotests: Test naming of throttling groups, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 21/34] qemu-iotests: Test setting WCE with qdev, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 23/34] qemu-img: Use strerror() for generic resize error, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 27/34] Improve block job rate limiting for small bandwidth values, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 32/34] hmp: show all of snapshot info on every block dev in output of 'info snapshots', Kevin Wolf, 2016/07/13