diff options
author | Eric Blake <eblake@redhat.com> | 2017-06-05 15:38:43 -0500 |
---|---|---|
committer | Michael Roth <mdroth@linux.vnet.ibm.com> | 2017-08-03 16:01:28 -0500 |
commit | 64945cb5f35f6b52ddc5ef0e4396b93d5bf87e1e (patch) | |
tree | 1c62d7c1ce6240a8da800082914bf0ca5edab0d0 /block | |
parent | 6a3f9c5c6e54d05e35bad786ce619d72d02c29de (diff) | |
download | qemu-64945cb5f35f6b52ddc5ef0e4396b93d5bf87e1e.tar.gz |
block: Guarantee that *file is set on bdrv_get_block_status()
We document that *file is valid if the return is not an error and
includes BDRV_BLOCK_OFFSET_VALID, but forgot to obey this contract
when a driver (such as blkdebug) lacks a callback. Messed up in
commit 67a0fd2 (v2.6), when we added the file parameter.
Enhance qemu-iotest 177 to cover this, using a sequence that would
print garbage or even SEGV, because it was dererefencing through
uninitialized memory. [The resulting test output shows that we
have less-than-ideal block status from the blkdebug driver, but
that's a separate fix coming up soon.]
Setting *file on all paths that return BDRV_BLOCK_OFFSET_VALID is
enough to fix the crash, but we can go one step further: always
setting *file, even on error, means that a broken caller that
blindly dereferences file without checking for error is now more
likely to get a reliable SEGV instead of randomly acting on garbage,
making it easier to diagnose such buggy callers. Adding an
assertion that file is set where expected doesn't hurt either.
CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 81c219ac6ce0d6182e35f3976f2caa4cefcaf9f0)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Diffstat (limited to 'block')
-rw-r--r-- | block/io.c | 5 |
1 files changed, 3 insertions, 2 deletions
diff --git a/block/io.c b/block/io.c index fe0c8677ba..a8589ee223 100644 --- a/block/io.c +++ b/block/io.c @@ -1757,6 +1757,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, int64_t n; int64_t ret, ret2; + *file = NULL; total_sectors = bdrv_nb_sectors(bs); if (total_sectors < 0) { return total_sectors; @@ -1777,11 +1778,11 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED; if (bs->drv->protocol_name) { ret |= BDRV_BLOCK_OFFSET_VALID | (sector_num * BDRV_SECTOR_SIZE); + *file = bs; } return ret; } - *file = NULL; bdrv_inc_in_flight(bs); ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum, file); @@ -1791,7 +1792,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, } if (ret & BDRV_BLOCK_RAW) { - assert(ret & BDRV_BLOCK_OFFSET_VALID); + assert(ret & BDRV_BLOCK_OFFSET_VALID && *file); ret = bdrv_co_get_block_status(*file, ret >> BDRV_SECTOR_BITS, *pnum, pnum, file); goto out; |