On 31.01.2017 17:09, Alberto Garcia wrote: > The type of qiov->size is size_t but the QEMU I/O code uses int all > over the place to measure request sizes. Overflows are likely going > to be detected by any of the many assert(qiov->size == nbytes), where > nbytes is an int, but let's add proper checks in the QEMUIOVector > initialization, which is where it belongs.
I'm not entirely convinced that's where it belongs. In my opinion, the BlockBackend functions should all get uint64_t size parameters and let blk_check_byte_request() do the rest. I wouldn't mind too much if blk_check_byte_requests() did assertions instead of returning -EIO. But the thing with the I/O vector is that it's not exclusively used for the block layer. I can see at least hw/9pfs/9p.c and hw/usb/core.c using it internally. The assertion probably makes sense even for them, considering that size_t does not have a constant size. But I'm not entirely sold that the I/O vector creation is actually the place where the assertions belong. Max > > Signed-off-by: Alberto Garcia <be...@igalia.com> > --- > util/iov.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/util/iov.c b/util/iov.c > index 74e6ca8ed7..6b5cc9203c 100644 > --- a/util/iov.c > +++ b/util/iov.c > @@ -283,13 +283,18 @@ void qemu_iovec_init_external(QEMUIOVector *qiov, > struct iovec *iov, int niov) > qiov->niov = niov; > qiov->nalloc = -1; > qiov->size = 0; > - for (i = 0; i < niov; i++) > + for (i = 0; i < niov; i++) { > + assert(iov[i].iov_len <= INT_MAX); > + assert(qiov->size <= INT_MAX - iov[i].iov_len); > qiov->size += iov[i].iov_len; > + } > } > > void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len) > { > assert(qiov->nalloc != -1); > + assert(len <= INT_MAX); > + assert(qiov->size <= INT_MAX - len); > > if (qiov->niov == qiov->nalloc) { > qiov->nalloc = 2 * qiov->nalloc + 1; >
signature.asc
Description: OpenPGP digital signature