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 <address@hidden>
---
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...