diff options
author | Kevin Wolf <kwolf@redhat.com> | 2014-03-28 18:06:31 +0100 |
---|---|---|
committer | Stefan Hajnoczi <stefanha@redhat.com> | 2014-04-01 15:21:03 +0200 |
commit | b106ad9185f35fc4ad669555ad0e79e276083bd7 (patch) | |
tree | 180dbb92f35c3299780f066ec023266ab90bc508 /block/qcow2.c | |
parent | 6d33e8e7dc9d40ea105feed4b39caa3e641569e8 (diff) |
qcow2: Don't rely on free_cluster_index in alloc_refcount_block() (CVE-2014-0147)
free_cluster_index is only correct if update_refcount() was called from
an allocation function, and even there it's brittle because it's used to
protect unfinished allocations which still have a refcount of 0 - if it
moves in the wrong place, the unfinished allocation can be corrupted.
So not using it any more seems to be a good idea. Instead, use the
first requested cluster to do the calculations. Return -EAGAIN if
unfinished allocations could become invalid and let the caller restart
its search for some free clusters.
The context of creating a snapsnot is one situation where
update_refcount() is called outside of a cluster allocation. For this
case, the change fixes a buffer overflow if a cluster is referenced in
an L2 table that cannot be represented by an existing refcount block.
(new_table[refcount_table_index] was out of bounds)
[Bump the qemu-iotests 026 refblock_alloc.write leak count from 10 to
11.
--Stefan]
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Diffstat (limited to 'block/qcow2.c')
-rw-r--r-- | block/qcow2.c | 11 |
1 files changed, 6 insertions, 5 deletions
diff --git a/block/qcow2.c b/block/qcow2.c index cc1bfebf29..d4d991cbb3 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1602,7 +1602,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, */ BlockDriverState* bs; QCowHeader *header; - uint8_t* refcount_table; + uint64_t* refcount_table; Error *local_err = NULL; int ret; @@ -1654,9 +1654,10 @@ static int qcow2_create2(const char *filename, int64_t total_size, goto out; } - /* Write an empty refcount table */ - refcount_table = g_malloc0(cluster_size); - ret = bdrv_pwrite(bs, cluster_size, refcount_table, cluster_size); + /* Write a refcount table with one refcount block */ + refcount_table = g_malloc0(2 * cluster_size); + refcount_table[0] = cpu_to_be64(2 * cluster_size); + ret = bdrv_pwrite(bs, cluster_size, refcount_table, 2 * cluster_size); g_free(refcount_table); if (ret < 0) { @@ -1681,7 +1682,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, goto out; } - ret = qcow2_alloc_clusters(bs, 2 * cluster_size); + ret = qcow2_alloc_clusters(bs, 3 * cluster_size); if (ret < 0) { error_setg_errno(errp, -ret, "Could not allocate clusters for qcow2 " "header and refcount table"); |