[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 04/17] block/io: use int64_t bytes in driver wrappers
From: |
Eric Blake |
Subject: |
Re: [PATCH v2 04/17] block/io: use int64_t bytes in driver wrappers |
Date: |
Wed, 29 Apr 2020 15:27:49 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
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. Convert driver wrappers parameters which are
already 64bit to signed type.
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
block/io.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index 1267918def..4796476835 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1086,7 +1086,7 @@ static void bdrv_co_io_em_complete(void *opaque, int ret)
}
static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
+ int64_t offset, int64_t bytes,
Remains 64 bits, the question is whether any caller could pass in
something larger than 63 bits. Callers are:
bdrv_co_do_copy_on_readv() - passes 'int64_t pnum', but sets pnum in a
fragmenting loop, MAX_BOUNCE_BUFFER when copy-on-read is needed, or
max_transfer bounded by BDRV_REQUEST_MAX_BYTES otherwise
bdrv_aligned_preadv() - passes 'unsigned int bytes' further limited by
fragmenting loop on max_transfer <= INT_MAX
Input is bounded to < 2G, internal use of 'bytes' is:
drv->bdrv_co_preadv_part(uint64_t) - safe
qemu_iovec_init_slice(size_t) - potential truncation on 32-bit platform,
if you don't fix qemu_iovec
drv->bdrv_co_preadv(uint64_t) - safe
drv->bdrv_aio_preadv(uint64_t) - safe
drv->bdrv_co_readv(int) after assertion of <2G - safe
QEMUIOVector *qiov,
size_t qiov_offset, int flags)
{
@@ -1155,7 +1155,7 @@ out:
}
static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
+ int64_t offset, int64_t bytes,
Remains 64 bits, the question is whether any caller could pass in
something larger than 63. Callers are:
bdrv_co_do_copy_on_readv() - passes 'int64_t pnum', but set in a
fragmenting loop bounded by MAX_BOUNCE_BUFFER
bdrv_co_do_pwrite_zeroes() - passes 'int num'
bdrv_aligned_pwritev() - passes 'unsigned int bytes' further limited by
fragmenting loop on max_transfer <= INT_MAX
Input is bounded to < 2G, internal use of 'bytes' is:
drv->bdrv_co_pwritev_part(uint64_t) - safe
qemu_iovec_init_slice(size_t) - potential truncation on 32-bit platform,
if you don't fix qemu_iovec
drv->bdrv_co_pwritev(uint64_t) - safe
drv->bdrv_aio_pwritev(uint64_t) - safe
drv->bdrv_co_writev(int) after assertion of <2G - safe
QEMUIOVector *qiov,
size_t qiov_offset, int flags)
{
@@ -1235,8 +1235,8 @@ emulate_flags:
}
static int coroutine_fn
-bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
+bdrv_driver_pwritev_compressed(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
size_t qiov_offset)
Remains 64 bits, the question is whether any caller could pass in
something larger than 63. Callers are:
bdrv_aligned_pwritev() - passes 'unsigned int bytes'
Input is bounded to < 4G, internal use of 'bytes' is:
drv->bdrv_co_pwritev_compressed_part(uint64_t) - safe
drv->bdrv_co_pwritev_compressed(uint64_t) - safe
qemu_iovec_init_slice(size_t) - potential truncation on 32-bit platform,
if you don't fix qemu_iovec
Because our input is bounded, all of the changes made here have no
semantic impact, and I argue this patch is safe. However, I also
pointed out the spots where we have latent bugs (on 32-bit machines
only) if the callers are relaxed to pass larger than 2G or 4G. As long
as you are auditing things, it would be nice to either fix that in this
patch, or document in the commit message how a future patch will address
that latent problem (whether by enforcing max_transfer to be capped at
2G on 32-bit platforms, or else fixing qemu_iovec to use int64_t instead
of size_t bounds).
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- Re: [PATCH v2 02/17] block: use int64_t as bytes type in tracked requests, (continued)
- [PATCH v2 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes(), Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 03/17] block/io: use int64_t bytes parameter in bdrv_check_byte_request(), Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev(), Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 08/17] block/io: support int64_t bytes in bdrv_aligned_preadv(), Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 07/17] block/io: support int64_t bytes in bdrv_co_do_copy_on_readv(), Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 09/17] block/io: support int64_t bytes in bdrv_co_p{read, write}v_part(), Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 04/17] block/io: use int64_t bytes in driver wrappers, Vladimir Sementsov-Ogievskiy, 2020/04/27
- Re: [PATCH v2 04/17] block/io: use int64_t bytes in driver wrappers,
Eric Blake <=
- [PATCH v2 10/17] block/io: support int64_t bytes in read/write wrappers, Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 13/17] block: use int64_t instead of uint64_t in driver read handlers, Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 14/17] block: use int64_t instead of uint64_t in driver write handlers, Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 16/17] block: use int64_t instead of int in driver write_zeroes handlers, Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 11/17] block/io: use int64_t bytes in copy_range, Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 17/17] block: use int64_t instead of int in driver discard handlers, Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 15/17] block: use int64_t instead of uint64_t in copy_range driver handlers, Vladimir Sementsov-Ogievskiy, 2020/04/27
- [PATCH v2 12/17] block/block-backend: convert blk io path to use int64_t parameters, Vladimir Sementsov-Ogievskiy, 2020/04/27
- Re: [PATCH v2 00/17] 64bit block-layer, no-reply, 2020/04/27
- Re: [PATCH v2 00/17] 64bit block-layer, no-reply, 2020/04/27