qed: Make qiov match request size until backing file EOF

If a QED image has a shorter backing file and a read request to
unallocated clusters goes across EOF of the backing file, the backing
file sees a shortened request and the rest is filled with zeros.
However, the original too long qiov was used with the shortened request.

This patch makes the qiov size match the request size, avoiding a
potential buffer overflow in raw-posix.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Kevin Wolf 2014-07-04 17:11:28 +02:00
parent 44deba5a52
commit f06ee3d4aa
2 changed files with 31 additions and 8 deletions

View File

@ -761,17 +761,19 @@ static BDRVQEDState *acb_to_s(QEDAIOCB *acb)
/** /**
* Read from the backing file or zero-fill if no backing file * Read from the backing file or zero-fill if no backing file
* *
* @s: QED state * @s: QED state
* @pos: Byte position in device * @pos: Byte position in device
* @qiov: Destination I/O vector * @qiov: Destination I/O vector
* @cb: Completion function * @backing_qiov: Possibly shortened copy of qiov, to be allocated here
* @opaque: User data for completion function * @cb: Completion function
* @opaque: User data for completion function
* *
* This function reads qiov->size bytes starting at pos from the backing file. * This function reads qiov->size bytes starting at pos from the backing file.
* If there is no backing file then zeroes are read. * If there is no backing file then zeroes are read.
*/ */
static void qed_read_backing_file(BDRVQEDState *s, uint64_t pos, static void qed_read_backing_file(BDRVQEDState *s, uint64_t pos,
QEMUIOVector *qiov, QEMUIOVector *qiov,
QEMUIOVector **backing_qiov,
BlockDriverCompletionFunc *cb, void *opaque) BlockDriverCompletionFunc *cb, void *opaque)
{ {
uint64_t backing_length = 0; uint64_t backing_length = 0;
@ -804,15 +806,21 @@ static void qed_read_backing_file(BDRVQEDState *s, uint64_t pos,
/* If the read straddles the end of the backing file, shorten it */ /* If the read straddles the end of the backing file, shorten it */
size = MIN((uint64_t)backing_length - pos, qiov->size); size = MIN((uint64_t)backing_length - pos, qiov->size);
assert(*backing_qiov == NULL);
*backing_qiov = g_new(QEMUIOVector, 1);
qemu_iovec_init(*backing_qiov, qiov->niov);
qemu_iovec_concat(*backing_qiov, qiov, 0, size);
BLKDBG_EVENT(s->bs->file, BLKDBG_READ_BACKING_AIO); BLKDBG_EVENT(s->bs->file, BLKDBG_READ_BACKING_AIO);
bdrv_aio_readv(s->bs->backing_hd, pos / BDRV_SECTOR_SIZE, bdrv_aio_readv(s->bs->backing_hd, pos / BDRV_SECTOR_SIZE,
qiov, size / BDRV_SECTOR_SIZE, cb, opaque); *backing_qiov, size / BDRV_SECTOR_SIZE, cb, opaque);
} }
typedef struct { typedef struct {
GenericCB gencb; GenericCB gencb;
BDRVQEDState *s; BDRVQEDState *s;
QEMUIOVector qiov; QEMUIOVector qiov;
QEMUIOVector *backing_qiov;
struct iovec iov; struct iovec iov;
uint64_t offset; uint64_t offset;
} CopyFromBackingFileCB; } CopyFromBackingFileCB;
@ -829,6 +837,12 @@ static void qed_copy_from_backing_file_write(void *opaque, int ret)
CopyFromBackingFileCB *copy_cb = opaque; CopyFromBackingFileCB *copy_cb = opaque;
BDRVQEDState *s = copy_cb->s; BDRVQEDState *s = copy_cb->s;
if (copy_cb->backing_qiov) {
qemu_iovec_destroy(copy_cb->backing_qiov);
g_free(copy_cb->backing_qiov);
copy_cb->backing_qiov = NULL;
}
if (ret) { if (ret) {
qed_copy_from_backing_file_cb(copy_cb, ret); qed_copy_from_backing_file_cb(copy_cb, ret);
return; return;
@ -866,11 +880,12 @@ static void qed_copy_from_backing_file(BDRVQEDState *s, uint64_t pos,
copy_cb = gencb_alloc(sizeof(*copy_cb), cb, opaque); copy_cb = gencb_alloc(sizeof(*copy_cb), cb, opaque);
copy_cb->s = s; copy_cb->s = s;
copy_cb->offset = offset; copy_cb->offset = offset;
copy_cb->backing_qiov = NULL;
copy_cb->iov.iov_base = qemu_blockalign(s->bs, len); copy_cb->iov.iov_base = qemu_blockalign(s->bs, len);
copy_cb->iov.iov_len = len; copy_cb->iov.iov_len = len;
qemu_iovec_init_external(&copy_cb->qiov, &copy_cb->iov, 1); qemu_iovec_init_external(&copy_cb->qiov, &copy_cb->iov, 1);
qed_read_backing_file(s, pos, &copy_cb->qiov, qed_read_backing_file(s, pos, &copy_cb->qiov, &copy_cb->backing_qiov,
qed_copy_from_backing_file_write, copy_cb); qed_copy_from_backing_file_write, copy_cb);
} }
@ -1313,7 +1328,7 @@ static void qed_aio_read_data(void *opaque, int ret,
return; return;
} else if (ret != QED_CLUSTER_FOUND) { } else if (ret != QED_CLUSTER_FOUND) {
qed_read_backing_file(s, acb->cur_pos, &acb->cur_qiov, qed_read_backing_file(s, acb->cur_pos, &acb->cur_qiov,
qed_aio_next_io, acb); &acb->backing_qiov, qed_aio_next_io, acb);
return; return;
} }
@ -1339,6 +1354,12 @@ static void qed_aio_next_io(void *opaque, int ret)
trace_qed_aio_next_io(s, acb, ret, acb->cur_pos + acb->cur_qiov.size); trace_qed_aio_next_io(s, acb, ret, acb->cur_pos + acb->cur_qiov.size);
if (acb->backing_qiov) {
qemu_iovec_destroy(acb->backing_qiov);
g_free(acb->backing_qiov);
acb->backing_qiov = NULL;
}
/* Handle I/O error */ /* Handle I/O error */
if (ret) { if (ret) {
qed_aio_complete(acb, ret); qed_aio_complete(acb, ret);
@ -1378,6 +1399,7 @@ static BlockDriverAIOCB *qed_aio_setup(BlockDriverState *bs,
acb->qiov_offset = 0; acb->qiov_offset = 0;
acb->cur_pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE; acb->cur_pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE;
acb->end_pos = acb->cur_pos + nb_sectors * BDRV_SECTOR_SIZE; acb->end_pos = acb->cur_pos + nb_sectors * BDRV_SECTOR_SIZE;
acb->backing_qiov = NULL;
acb->request.l2_table = NULL; acb->request.l2_table = NULL;
qemu_iovec_init(&acb->cur_qiov, qiov->niov); qemu_iovec_init(&acb->cur_qiov, qiov->niov);

View File

@ -142,6 +142,7 @@ typedef struct QEDAIOCB {
/* Current cluster scatter-gather list */ /* Current cluster scatter-gather list */
QEMUIOVector cur_qiov; QEMUIOVector cur_qiov;
QEMUIOVector *backing_qiov;
uint64_t cur_pos; /* position on block device, in bytes */ uint64_t cur_pos; /* position on block device, in bytes */
uint64_t cur_cluster; /* cluster offset in image file */ uint64_t cur_cluster; /* cluster offset in image file */
unsigned int cur_nclusters; /* number of clusters being accessed */ unsigned int cur_nclusters; /* number of clusters being accessed */