mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

block: Guarantee that *file is set on bdrv_get_block_status()

Browse Source 
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>
This commit is contained in:
Eric Blake 2017-06-05 15:38:43 -05:00 committed by Kevin Wolf
parent 64ebf55648
commit 81c219ac6c
3 changed files with 8 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
block/io.c
View File

@ -1734,6 +1734,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;
@ -1757,11 +1758,11 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
}
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);
@ -1771,7 +1772,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;

3
tests/qemu-iotests/177
View File

@ -43,6 +43,7 @@ _supported_proto file
CLUSTER_SIZE=1M
size=128M
options=driver=blkdebug,image.driver=qcow2
nested_opts=image.file.driver=file,image.file.filename=$TEST_IMG
echo
echo "== setting up files =="
@ -106,6 +107,8 @@ function verify_io()
}
verify_io | $QEMU_IO -r "$TEST_IMG" | _filter_qemu_io
$QEMU_IMG map --image-opts "$options,$nested_opts,align=4k" \
| _filter_qemu_img_map
_check_test_img

2
tests/qemu-iotests/177.out
View File

@ -45,5 +45,7 @@ read 30408704/30408704 bytes at offset 80740352
29 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 23068672/23068672 bytes at offset 111149056
22 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0 0x8000000 json:{"image": {"driver": "IMGFMT", "file": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}}, "driver": "blkdebug", "align": "4k"}
No errors were found on the image.
*** done