From d7a753a148d4738eef95a3b193b081a9c905399f Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 9 Aug 2017 15:38:06 -0500 Subject: [PATCH] qcow: Check failure of bdrv_getlength() and bdrv_truncate() Omitting the check for whether bdrv_getlength() and bdrv_truncate() failed meant that it was theoretically possible to return an incorrect offset to the caller. More likely, conditions for either of these functions to fail would also cause one of our other calls (such as bdrv_pread() or bdrv_pwrite_sync()) to also fail, but auditing that we are safe is difficult compared to just patching things to always forward on the error rather than ignoring it. Use osdep.h macros instead of open-coded rounding while in the area. Reported-by: Markus Armbruster Signed-off-by: Eric Blake Signed-off-by: Kevin Wolf --- block/qcow.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index d07bef6306..f450b00cfc 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -357,7 +357,8 @@ static int get_cluster_offset(BlockDriverState *bs, { BDRVQcowState *s = bs->opaque; int min_index, i, j, l1_index, l2_index, ret; - uint64_t l2_offset, *l2_table, cluster_offset, tmp; + int64_t l2_offset; + uint64_t *l2_table, cluster_offset, tmp; uint32_t min_count; int new_l2_table; @@ -370,8 +371,11 @@ static int get_cluster_offset(BlockDriverState *bs, return 0; /* allocate a new l2 entry */ l2_offset = bdrv_getlength(bs->file->bs); + if (l2_offset < 0) { + return l2_offset; + } /* round to cluster size */ - l2_offset = (l2_offset + s->cluster_size - 1) & ~(s->cluster_size - 1); + l2_offset = QEMU_ALIGN_UP(l2_offset, s->cluster_size); /* update the L1 entry */ s->l1_table[l1_index] = l2_offset; tmp = cpu_to_be64(l2_offset); @@ -438,8 +442,10 @@ static int get_cluster_offset(BlockDriverState *bs, return -EIO; } cluster_offset = bdrv_getlength(bs->file->bs); - cluster_offset = (cluster_offset + s->cluster_size - 1) & - ~(s->cluster_size - 1); + if ((int64_t) cluster_offset < 0) { + return cluster_offset; + } + cluster_offset = QEMU_ALIGN_UP(cluster_offset, s->cluster_size); /* write the cluster content */ ret = bdrv_pwrite(bs->file, cluster_offset, s->cluster_cache, s->cluster_size); @@ -448,12 +454,20 @@ static int get_cluster_offset(BlockDriverState *bs, } } else { cluster_offset = bdrv_getlength(bs->file->bs); + if ((int64_t) cluster_offset < 0) { + return cluster_offset; + } if (allocate == 1) { /* round to cluster size */ - cluster_offset = (cluster_offset + s->cluster_size - 1) & - ~(s->cluster_size - 1); - bdrv_truncate(bs->file, cluster_offset + s->cluster_size, - PREALLOC_MODE_OFF, NULL); + cluster_offset = QEMU_ALIGN_UP(cluster_offset, s->cluster_size); + if (cluster_offset + s->cluster_size > INT64_MAX) { + return -E2BIG; + } + ret = bdrv_truncate(bs->file, cluster_offset + s->cluster_size, + PREALLOC_MODE_OFF, NULL); + if (ret < 0) { + return ret; + } /* if encrypted, we must initialize the cluster content which won't be written */ if (bs->encrypted &&