* [Qemu-devel] [PATCH v3 0/2] qcow2: Make size_to_clusters() return uint64_t
@ 2015-09-14 14:39 Max Reitz
2015-09-14 14:39 ` [Qemu-devel] [PATCH v3 1/2] " Max Reitz
2015-09-14 14:39 ` [Qemu-devel] [PATCH v3 2/2] iotests: Add test for checking large image files Max Reitz
0 siblings, 2 replies; 3+ messages in thread
From: Max Reitz @ 2015-09-14 14:39 UTC (permalink / raw
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Max Reitz
Some callers actually expected that function to return uint64_t. As it
turns out, it doesn't. Fix that.
v3:
- Patch 1: Fix build for sizeof(void *) < 8 machines [Peter]
git-backport-diff against v2:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/2:[0002] [FC] 'qcow2: Make size_to_clusters() return uint64_t'
002/2:[----] [--] 'iotests: Add test for checking large image files'
Max Reitz (2):
qcow2: Make size_to_clusters() return uint64_t
iotests: Add test for checking large image files
block/qcow2-cluster.c | 30 ++++++++++++-------
block/qcow2-refcount.c | 12 +++++---
block/qcow2.h | 6 ++--
tests/qemu-iotests/138 | 73 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/138.out | 9 ++++++
tests/qemu-iotests/group | 1 +
6 files changed, 113 insertions(+), 18 deletions(-)
create mode 100755 tests/qemu-iotests/138
create mode 100644 tests/qemu-iotests/138.out
--
2.5.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Qemu-devel] [PATCH v3 1/2] qcow2: Make size_to_clusters() return uint64_t
2015-09-14 14:39 [Qemu-devel] [PATCH v3 0/2] qcow2: Make size_to_clusters() return uint64_t Max Reitz
@ 2015-09-14 14:39 ` Max Reitz
2015-09-14 14:39 ` [Qemu-devel] [PATCH v3 2/2] iotests: Add test for checking large image files Max Reitz
1 sibling, 0 replies; 3+ messages in thread
From: Max Reitz @ 2015-09-14 14:39 UTC (permalink / raw
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Max Reitz
Sadly, some images may have more clusters than what can be represented
using a plain int. We should be prepared for that case (in
qcow2_check_refcounts() we actually were trying to catch that case, but
since size_to_clusters() truncated the returned value, that check never
did anything useful).
Cc: qemu-stable <qemu-stable@nongnu.org>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-cluster.c | 30 +++++++++++++++++++-----------
block/qcow2-refcount.c | 12 ++++++++----
block/qcow2.h | 6 +++---
3 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 2975b83..dc217e7 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -298,7 +298,7 @@ fail:
* as contiguous. (This allows it, for example, to stop at the first compressed
* cluster which may require a different handling)
*/
-static int count_contiguous_clusters(uint64_t nb_clusters, int cluster_size,
+static int count_contiguous_clusters(int nb_clusters, int cluster_size,
uint64_t *l2_table, uint64_t stop_flags)
{
int i;
@@ -321,7 +321,7 @@ static int count_contiguous_clusters(uint64_t nb_clusters, int cluster_size,
return i;
}
-static int count_contiguous_free_clusters(uint64_t nb_clusters, uint64_t *l2_table)
+static int count_contiguous_free_clusters(int nb_clusters, uint64_t *l2_table)
{
int i;
@@ -495,6 +495,7 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
if (nb_needed > nb_available) {
nb_needed = nb_available;
}
+ assert(nb_needed <= INT_MAX);
*cluster_offset = 0;
@@ -530,6 +531,8 @@ 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, nb_needed << 9);
ret = qcow2_get_cluster_type(*cluster_offset);
@@ -840,7 +843,7 @@ err:
static int count_cow_clusters(BDRVQcow2State *s, int nb_clusters,
uint64_t *l2_table, int l2_index)
{
- int i;
+ uint64_t i;
for (i = 0; i < nb_clusters; i++) {
uint64_t l2_entry = be64_to_cpu(l2_table[l2_index + i]);
@@ -960,7 +963,7 @@ static int handle_copied(BlockDriverState *bs, uint64_t guest_offset,
int l2_index;
uint64_t cluster_offset;
uint64_t *l2_table;
- unsigned int nb_clusters;
+ uint64_t nb_clusters;
unsigned int keep_clusters;
int ret;
@@ -979,6 +982,7 @@ static int handle_copied(BlockDriverState *bs, uint64_t guest_offset,
l2_index = offset_to_l2_index(s, guest_offset);
nb_clusters = MIN(nb_clusters, s->l2_size - l2_index);
+ assert(nb_clusters <= INT_MAX);
/* Find L2 entry for the first involved cluster */
ret = get_cluster_table(bs, guest_offset, &l2_table, &l2_index);
@@ -1061,7 +1065,7 @@ out:
* restarted, but the whole request should not be failed.
*/
static int do_alloc_cluster_offset(BlockDriverState *bs, uint64_t guest_offset,
- uint64_t *host_offset, unsigned int *nb_clusters)
+ uint64_t *host_offset, uint64_t *nb_clusters)
{
BDRVQcow2State *s = bs->opaque;
@@ -1079,7 +1083,7 @@ static int do_alloc_cluster_offset(BlockDriverState *bs, uint64_t guest_offset,
*host_offset = cluster_offset;
return 0;
} else {
- int ret = qcow2_alloc_clusters_at(bs, *host_offset, *nb_clusters);
+ int64_t ret = qcow2_alloc_clusters_at(bs, *host_offset, *nb_clusters);
if (ret < 0) {
return ret;
}
@@ -1115,7 +1119,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t guest_offset,
int l2_index;
uint64_t *l2_table;
uint64_t entry;
- unsigned int nb_clusters;
+ uint64_t nb_clusters;
int ret;
uint64_t alloc_cluster_offset;
@@ -1133,6 +1137,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t guest_offset,
l2_index = offset_to_l2_index(s, guest_offset);
nb_clusters = MIN(nb_clusters, s->l2_size - l2_index);
+ assert(nb_clusters <= INT_MAX);
/* Find L2 entry for the first involved cluster */
ret = get_cluster_table(bs, guest_offset, &l2_table, &l2_index);
@@ -1426,7 +1431,8 @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
* clusters.
*/
static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
- unsigned int nb_clusters, enum qcow2_discard_type type, bool full_discard)
+ uint64_t nb_clusters, enum qcow2_discard_type type,
+ bool full_discard)
{
BDRVQcow2State *s = bs->opaque;
uint64_t *l2_table;
@@ -1441,6 +1447,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
/* Limit nb_clusters to one L2 table */
nb_clusters = MIN(nb_clusters, s->l2_size - l2_index);
+ assert(nb_clusters <= INT_MAX);
for (i = 0; i < nb_clusters; i++) {
uint64_t old_l2_entry;
@@ -1503,7 +1510,7 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
{
BDRVQcow2State *s = bs->opaque;
uint64_t end_offset;
- unsigned int nb_clusters;
+ uint64_t nb_clusters;
int ret;
end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);
@@ -1545,7 +1552,7 @@ fail:
* clusters.
*/
static int zero_single_l2(BlockDriverState *bs, uint64_t offset,
- unsigned int nb_clusters)
+ uint64_t nb_clusters)
{
BDRVQcow2State *s = bs->opaque;
uint64_t *l2_table;
@@ -1560,6 +1567,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t offset,
/* Limit nb_clusters to one L2 table */
nb_clusters = MIN(nb_clusters, s->l2_size - l2_index);
+ assert(nb_clusters <= INT_MAX);
for (i = 0; i < nb_clusters; i++) {
uint64_t old_offset;
@@ -1584,7 +1592,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t offset,
int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors)
{
BDRVQcow2State *s = bs->opaque;
- unsigned int nb_clusters;
+ uint64_t nb_clusters;
int ret;
/* The zero flag is only supported by version 3 and newer */
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 53e3e9f..d9f234a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -875,8 +875,8 @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t size)
return offset;
}
-int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
- int nb_clusters)
+int64_t qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
+ int64_t nb_clusters)
{
BDRVQcow2State *s = bs->opaque;
uint64_t cluster_index, refcount;
@@ -1259,7 +1259,7 @@ static size_t refcount_array_byte_size(BDRVQcow2State *s, uint64_t entries)
static int realloc_refcount_array(BDRVQcow2State *s, void **array,
int64_t *size, int64_t new_size)
{
- size_t old_byte_size, new_byte_size;
+ int64_t old_byte_size, new_byte_size;
void *new_ptr;
/* Round to clusters so the array can be directly written to disk */
@@ -1275,13 +1275,17 @@ static int realloc_refcount_array(BDRVQcow2State *s, void **array,
assert(new_byte_size > 0);
+ if (new_byte_size > SIZE_MAX) {
+ return -ENOMEM;
+ }
+
new_ptr = g_try_realloc(*array, new_byte_size);
if (!new_ptr) {
return -ENOMEM;
}
if (new_byte_size > old_byte_size) {
- memset((void *)((uintptr_t)new_ptr + old_byte_size), 0,
+ memset((char *)new_ptr + old_byte_size, 0,
new_byte_size - old_byte_size);
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 61f1b57..d700bf1 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -415,7 +415,7 @@ static inline int64_t offset_into_cluster(BDRVQcow2State *s, int64_t offset)
return offset & (s->cluster_size - 1);
}
-static inline int size_to_clusters(BDRVQcow2State *s, int64_t size)
+static inline uint64_t size_to_clusters(BDRVQcow2State *s, uint64_t size)
{
return (size + (s->cluster_size - 1)) >> s->cluster_bits;
}
@@ -509,8 +509,8 @@ int qcow2_update_cluster_refcount(BlockDriverState *bs, int64_t cluster_index,
enum qcow2_discard_type type);
int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t size);
-int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
- int nb_clusters);
+int64_t qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
+ int64_t nb_clusters);
int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size);
void qcow2_free_clusters(BlockDriverState *bs,
int64_t offset, int64_t size,
--
2.5.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] iotests: Add test for checking large image files
2015-09-14 14:39 [Qemu-devel] [PATCH v3 0/2] qcow2: Make size_to_clusters() return uint64_t Max Reitz
2015-09-14 14:39 ` [Qemu-devel] [PATCH v3 1/2] " Max Reitz
@ 2015-09-14 14:39 ` Max Reitz
1 sibling, 0 replies; 3+ messages in thread
From: Max Reitz @ 2015-09-14 14:39 UTC (permalink / raw
To: qemu-block; +Cc: Kevin Wolf, qemu-devel, Max Reitz
Add a test for checking a qcow2 file with a multiple of 2^32 clusters.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/qemu-iotests/138 | 73 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/138.out | 9 ++++++
tests/qemu-iotests/group | 1 +
3 files changed, 83 insertions(+)
create mode 100755 tests/qemu-iotests/138
create mode 100644 tests/qemu-iotests/138.out
diff --git a/tests/qemu-iotests/138 b/tests/qemu-iotests/138
new file mode 100755
index 0000000..a5c3464
--- /dev/null
+++ b/tests/qemu-iotests/138
@@ -0,0 +1,73 @@
+#!/bin/bash
+#
+# General test case for qcow2's image check
+#
+# Copyright (C) 2015 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"
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This tests qocw2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+echo
+echo '=== Check on an image with a multiple of 2^32 clusters ==='
+echo
+
+IMGOPTS=$(_optstr_add "$IMGOPTS" "cluster_size=512") \
+ _make_test_img 512
+
+# Allocate L2 table
+$QEMU_IO -c 'write 0 512' "$TEST_IMG" | _filter_qemu_io
+
+# Put the data cluster at a multiple of 2 TB, resulting in the image apparently
+# having a multiple of 2^32 clusters
+# (To be more specific: It is at 32 PB)
+poke_file "$TEST_IMG" 2048 "\x80\x80\x00\x00\x00\x00\x00\x00"
+
+# An offset of 32 PB results in qemu-img check having to allocate an in-memory
+# refcount table of 128 TB (16 bit refcounts, 512 byte clusters).
+# This should be generally too much for any system and thus fail.
+# What this test is checking is that the qcow2 driver actually tries to allocate
+# such a large amount of memory (and is consequently aborting) instead of having
+# truncated the cluster count somewhere (which would result in much less memory
+# being allocated and then a segfault occurring).
+_check_test_img
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/138.out b/tests/qemu-iotests/138.out
new file mode 100644
index 0000000..3fe911f
--- /dev/null
+++ b/tests/qemu-iotests/138.out
@@ -0,0 +1,9 @@
+QA output created by 138
+
+=== Check on an image with a multiple of 2^32 clusters ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=512
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: Check failed: Cannot allocate memory
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 3a6a8f0..439b1d2 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -135,3 +135,4 @@
134 rw auto quick
135 rw auto
137 rw auto
+138 rw auto quick
--
2.5.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-09-14 14:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-14 14:39 [Qemu-devel] [PATCH v3 0/2] qcow2: Make size_to_clusters() return uint64_t Max Reitz
2015-09-14 14:39 ` [Qemu-devel] [PATCH v3 1/2] " Max Reitz
2015-09-14 14:39 ` [Qemu-devel] [PATCH v3 2/2] iotests: Add test for checking large image files Max Reitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).