On Wed, May 13, 2015 at 01:11:58PM +0000, Fam Zheng wrote: > v7: Add Kevin's rev-by in patch 1 and 3. > Address Stefan's and Kevin's comments on patch 2: > - Don't duplicate tracked_request_begin and tracked_request_end; > - Don't forget to remove debug printf; > - Call qemu_vfree unconditionally; > - Don't serialize aligned part of the zero write req; > > An unaligned zero write causes NULL deferencing in bdrv_co_do_pwritev. That > path is reachable from bdrv_co_write_zeroes and bdrv_aio_write_zeroes. > > You can easily trigger through the former with qemu-io, as the test case added > by 61815d6e0aa. For bdrv_aio_write_zeroes, in common cases there's always a > format driver (which uses 512 alignment), so it would be much rarer to have > unaligned requests (only concerning top level here, when the request goes down > to bs->file, where for example the alignment is 4k, it would then be calling > bdrv_co_write_zeroes because it's in a coroutine). > > fc3959e4669a1c fixed bdrv_co_write_zeroes but not bdrv_aio_write_zeroes. The > lattern is the actually used one by device model. Revert the previous fix, do > it in bdrv_co_do_pwritev, to cover both paths. > > > > Fam Zheng (3): > Revert "block: Fix unaligned zero write" > block: Fix NULL deference for unaligned write if qiov is NULL > qemu-iotests: Test unaligned sub-block zero write > > block/io.c | 142 > ++++++++++++++++++++++++++++++++------------- > tests/qemu-iotests/033 | 13 +++++ > tests/qemu-iotests/033.out | 30 ++++++++++ > 3 files changed, 144 insertions(+), 41 deletions(-)
By the way, gcc 4.9.2 says: block/io.c: In function ‘bdrv_co_do_pwritev’: block/io.c:1193:9: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized] int ret; ^ I have squashed a fix. Stefan
pgp3gCqXcKu8N.pgp
Description: PGP signature