file-posix: Better checks of 64-bit copy_range
PATCH 1/1 file-posix: Better checks of 64-bit copy_range
file-posix.c was taking a 64-bit bytes in raw_co_copy_range_to(), passing it through a 32-bit parameter of paio_submit_co_full(), then widening it back to size_t when assigning into acb->aio_nbytes.
Looking at io.c, I can't quickly tell if bdrv_co_copy_range_internal() is fragmenting things to honor bs->bl.max_transfer, or if it can accidentally send requests larger than 2G down to the driver. If the former, then this is a no-op; if the latter, then someone needs to find a way to trigger this assertion and patch the block layer to properly fragment copy_range requests. Either way, we're better off with an assertion than the risk of silent data corruption.
--- block/file-posix.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
block/file-posix.c +3 -2
@@ -1821,7 +1821,7 @@ static int aio_worker(void *arg)static int paio_submit_co_full (BlockDriverState *bs,int fd,int64_t offset,int fd2,int64_t offset2,QEMUIOVector *qiov, -int bytes,int type) +uint64_t bytes,int type) {RawPosixAIOData *acb =g_new (RawPosixAIOData,1 );ThreadPool *pool;
@@ -1832,6 +1832,7 @@ static int paio_submit_co_full(BlockDriverState *bs, int fd, acb->aio_fd2 = fd2; acb->aio_offset2 = offset2; +assert (bytes <=SIZE_MAX ); acb->aio_nbytes = bytes; acb->aio_offset = offset;
@@ -1848,7 +1849,7 @@ static int paio_submit_co_full(BlockDriverState *bs, int fd,static inline int paio_submit_co (BlockDriverState *bs,int fd,int64_t offset,QEMUIOVector *qiov, -int bytes,int type) +uint64_t bytes,int type) {return paio_submit_co_full (bs, fd, offset,-1 ,0 , qiov, bytes, type); }
Signature · 1 line
2.17.2
Quoted 12 lines · Eric Blake
file-posix.cwas taking a 64-bit bytes inraw_co_copy_range_to(), passing it through a 32-bit parameter ofpaio_submit_co_full(), then widening it back tosize_twhen assigning intoacb->aio_nbytes.Looking at
io.c, I can't quickly tell ifbdrv_co_copy_range_internal()is fragmenting things to honorbs->bl.max_transfer, or if it can accidentally send requests larger than 2G down to the driver. If the former, then this is a no-op; if the latter, then someone needs to find a way to trigger this assertion and patch the block layer to properly fragmentcopy_rangerequests. Either way, we're better off with an assertion than the risk of silent data corruption.
Quoted 4 lines
Signed-off-by Eric Blake <[email protected]>--- block/file-posix.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
This patch is not needed after Kevin's file-posix cleanups for post-release:
https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02985.html
The question remains, though, if we still want this in 3.1.
Quoted 32 lines
block/file-posix.c +3
-2@@ -1821,7 +1821,7 @@ static int aio_worker(void *arg)static int paio_submit_co_full (BlockDriverState *bs,int fd,int64_t offset,int fd2,int64_t offset2,QEMUIOVector *qiov, -int bytes,int type) +uint64_t bytes,int type) {RawPosixAIOData *acb =g_new (RawPosixAIOData,1 );ThreadPool *pool;@@ -1832,6 +1832,7 @@ static int paio_submit_co_full(BlockDriverState *bs, int fd, acb->aio_fd2 = fd2; acb->aio_offset2 = offset2; +assert (bytes <=SIZE_MAX ); acb->aio_nbytes = bytes; acb->aio_offset = offset;@@ -1848,7 +1849,7 @@ static int paio_submit_co_full(BlockDriverState *bs, int fd,static inline int paio_submit_co (BlockDriverState *bs,int fd,int64_t offset,QEMUIOVector *qiov, -int bytes,int type) +uint64_t bytes,int type) {return paio_submit_co_full (bs, fd, offset,-1 ,0 , qiov, bytes, type); }
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org