From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] [PULL 20/23] qcow2: Make size_to_clusters() return uint64_t
Date: Fri, 11 Sep 2015 21:41:00 +0200 [thread overview]
Message-ID: <1442000463-22777-21-git-send-email-kwolf@redhat.com> (raw)
In-Reply-To: <1442000463-22777-1-git-send-email-kwolf@redhat.com>
From: Max Reitz <mreitz@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2-cluster.c | 28 ++++++++++++++++++----------
block/qcow2-refcount.c | 10 +++++++---
block/qcow2.h | 6 +++---
3 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 412ee27..6ede629 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);
@@ -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 b780bb9..5f67798 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,6 +1275,10 @@ 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;
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,
--
1.8.3.1
next prev parent reply other threads:[~2015-09-11 19:48 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-11 19:40 [Qemu-devel] [PULL 00/23] Block layer patches Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 01/23] block: Always pass NULL as drv for bdrv_open() Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 02/23] block: Drop drv parameter from bdrv_open() Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 03/23] block: Drop drv parameter from bdrv_open_inherit() Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 04/23] block: Drop drv parameter from bdrv_fill_options() Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 05/23] block: Drop bdrv_find_whitelisted_format() Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 06/23] qcow2: Rename BDRVQcowState to BDRVQcow2State Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 07/23] block: Allow specifying driver-specific options to reopen Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 08/23] qemu-io: Remove duplicate 'open' error message Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 09/23] qemu-io: Add command 'reopen' Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 10/23] qcow2: Improve error message Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 11/23] qcow2: Factor out qcow2_update_options() Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 12/23] qcow2: Move qcow2_update_options() call up Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 13/23] qcow2: Move rest of option handling to qcow2_update_options() Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 14/23] qcow2: Leave s unchanged on qcow2_update_options() failure Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 15/23] qcow2: Fix memory leak in qcow2_update_options() error path Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 16/23] qcow2: Make qcow2_update_options() suitable for transactions Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 17/23] qcow2: Support updating driver-specific options in reopen Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 18/23] qemu-iotests: Reopen qcow2 with lazy-refcounts change Kevin Wolf
2015-09-11 19:40 ` [Qemu-devel] [PULL 19/23] qemu-iotests: More qcow2 reopen tests Kevin Wolf
2015-09-11 19:41 ` Kevin Wolf [this message]
2015-09-11 19:41 ` [Qemu-devel] [PULL 21/23] iotests: Add test for checking large image files Kevin Wolf
2015-09-11 19:41 ` [Qemu-devel] [PULL 22/23] vmdk: Fix next_cluster_sector for compressed write Kevin Wolf
2015-09-11 19:41 ` [Qemu-devel] [PULL 23/23] qcow2: Make qcow2_alloc_bytes() more explicit Kevin Wolf
2015-09-14 9:46 ` [Qemu-devel] [PULL 00/23] Block layer patches Peter Maydell
2015-09-14 9:57 ` Kevin Wolf
2015-09-14 14:36 ` Max Reitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1442000463-22777-21-git-send-email-kwolf@redhat.com \
--to=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).