From patchwork Tue Jul 8 13:14:59 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 368799 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 8C38B14011B for ; Fri, 11 Jul 2014 07:42:20 +1000 (EST) Received: from localhost ([::1]:58552 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4h04-0007z2-63 for incoming@patchwork.ozlabs.org; Tue, 08 Jul 2014 21:48:48 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38259) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4VEk-000442-44 for qemu-devel@nongnu.org; Tue, 08 Jul 2014 09:15:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X4VEd-00046c-S1 for qemu-devel@nongnu.org; Tue, 08 Jul 2014 09:15:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:20609) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4VEd-00044G-I9 for qemu-devel@nongnu.org; Tue, 08 Jul 2014 09:15:03 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s68DF2Q2011778 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Tue, 8 Jul 2014 09:15:03 -0400 Received: from noname.str.redhat.com (dhcp-192-197.str.redhat.com [10.33.192.197]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s68DF0cR019817; Tue, 8 Jul 2014 09:15:01 -0400 From: Kevin Wolf To: qemu-devel@nongnu.org Date: Tue, 8 Jul 2014 15:14:59 +0200 Message-Id: <1404825299-2827-1-git-send-email-kwolf@redhat.com> In-Reply-To: <53B85AD2.2090706@redhat.com> References: <53B85AD2.2090706@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, stefanha@redhat.com, mreitz@redhat.com Subject: [Qemu-devel] [PATCH v2 for-2.1 3/4] qed: Make qiov match request size until backing file EOF X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 --- block/qed.c | 38 ++++++++++++++++++++++++++++++-------- block/qed.h | 1 + 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/block/qed.c b/block/qed.c index b69374b..cd4872b 100644 --- a/block/qed.c +++ b/block/qed.c @@ -761,17 +761,19 @@ static BDRVQEDState *acb_to_s(QEDAIOCB *acb) /** * Read from the backing file or zero-fill if no backing file * - * @s: QED state - * @pos: Byte position in device - * @qiov: Destination I/O vector - * @cb: Completion function - * @opaque: User data for completion function + * @s: QED state + * @pos: Byte position in device + * @qiov: Destination I/O vector + * @backing_qiov: Possibly shortened copy of qiov, to be allocated here + * @cb: Completion function + * @opaque: User data for completion function * * This function reads qiov->size bytes starting at pos from the backing file. * If there is no backing file then zeroes are read. */ static void qed_read_backing_file(BDRVQEDState *s, uint64_t pos, QEMUIOVector *qiov, + QEMUIOVector **backing_qiov, BlockDriverCompletionFunc *cb, void *opaque) { 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 */ 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); 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 { GenericCB gencb; BDRVQEDState *s; QEMUIOVector qiov; + QEMUIOVector *backing_qiov; struct iovec iov; uint64_t offset; } CopyFromBackingFileCB; @@ -829,6 +837,12 @@ static void qed_copy_from_backing_file_write(void *opaque, int ret) CopyFromBackingFileCB *copy_cb = opaque; 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) { qed_copy_from_backing_file_cb(copy_cb, ret); 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->s = s; copy_cb->offset = offset; + copy_cb->backing_qiov = NULL; copy_cb->iov.iov_base = qemu_blockalign(s->bs, len); copy_cb->iov.iov_len = len; qemu_iovec_init_external(©_cb->qiov, ©_cb->iov, 1); - qed_read_backing_file(s, pos, ©_cb->qiov, + qed_read_backing_file(s, pos, ©_cb->qiov, ©_cb->backing_qiov, qed_copy_from_backing_file_write, copy_cb); } @@ -1313,7 +1328,7 @@ static void qed_aio_read_data(void *opaque, int ret, return; } else if (ret != QED_CLUSTER_FOUND) { 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; } @@ -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); + if (acb->backing_qiov) { + qemu_iovec_destroy(acb->backing_qiov); + g_free(acb->backing_qiov); + acb->backing_qiov = NULL; + } + /* Handle I/O error */ if (ret) { qed_aio_complete(acb, ret); @@ -1378,6 +1399,7 @@ static BlockDriverAIOCB *qed_aio_setup(BlockDriverState *bs, acb->qiov_offset = 0; acb->cur_pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE; acb->end_pos = acb->cur_pos + nb_sectors * BDRV_SECTOR_SIZE; + acb->backing_qiov = NULL; acb->request.l2_table = NULL; qemu_iovec_init(&acb->cur_qiov, qiov->niov); diff --git a/block/qed.h b/block/qed.h index b024751..2b0e724 100644 --- a/block/qed.h +++ b/block/qed.h @@ -142,6 +142,7 @@ typedef struct QEDAIOCB { /* Current cluster scatter-gather list */ QEMUIOVector cur_qiov; + QEMUIOVector *backing_qiov; uint64_t cur_pos; /* position on block device, in bytes */ uint64_t cur_cluster; /* cluster offset in image file */ unsigned int cur_nclusters; /* number of clusters being accessed */