On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
dependencies: bdrv_co_write_req_prepare() and
bdrv_co_write_req_finish() to signed type bytes)
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
block/io.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/io.c b/block/io.c
index c8c30e3699..fe19e09034 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1854,7 +1854,7 @@ fail:
}
static inline int coroutine_fn
-bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
+bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
BdrvTrackedRequest *req, int flags)
{
No change in size. First, check usage within function:
int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
Changes computation from uint64_t to int64_t. This causes a borderline
bug on images between INT64_MAX-511 and INT64_MAX (nbdkit can produce
such images over NBD, although they are atypical on disk), where
DIV_ROUND_UP() would give the right answer as uint64_t but a negative
answer with int64_t. As those images are not sector-aligned, maybe we
don't need to care?
all other uses appear to be within asserts related to offset+bytes being
positive, so that's what we should check for.
Callers:
bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed
below [1]
bdrv_co_pdiscard() - already passes 'int64_t', also checks for
offset+bytes overflow - safe
bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for
3/17 how it was capped < 2M - safe
bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed
by subtracting from a positive 'int64_t offset' - safe
[1] except I hit the end of my work day, so my analysis will have to
continue tomorrow...
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org