From nobody Thu May 2 11:45:45 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1511365078371125.39066670187253; Wed, 22 Nov 2017 07:37:58 -0800 (PST) Received: from localhost ([::1]:40073 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eHX5y-0002NX-HW for importer@patchew.org; Wed, 22 Nov 2017 10:37:50 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59668) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eHX3w-0000vA-Tw for qemu-devel@nongnu.org; Wed, 22 Nov 2017 10:35:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eHX3s-0001Rz-QY for qemu-devel@nongnu.org; Wed, 22 Nov 2017 10:35:44 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:39382) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eHX3i-0001QI-4u; Wed, 22 Nov 2017 10:35:30 -0500 Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id vAMFZQ9O014866 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 22 Nov 2017 15:35:27 GMT Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by aserv0021.oracle.com (8.14.4/8.14.4) with ESMTP id vAMFZQDb022329 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 22 Nov 2017 15:35:26 GMT Received: from abhmp0015.oracle.com (abhmp0015.oracle.com [141.146.116.21]) by userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id vAMFZPi5026026; Wed, 22 Nov 2017 15:35:25 GMT Received: from deepas-x1.oracle.com (/10.197.190.204) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 22 Nov 2017 07:35:25 -0800 From: Deepa Srinivasan To: kwolf@redhat.com, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, mark.kanda@oracle.com Date: Wed, 22 Nov 2017 07:33:28 -0800 Message-Id: <1511364808-30171-1-git-send-email-deepa.srinivasan@oracle.com> X-Mailer: git-send-email 2.7.4 X-Source-IP: aserv0021.oracle.com [141.146.126.233] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] [fuzzy] X-Received-From: 156.151.31.81 Subject: [Qemu-devel] [PATCH] block: Fix qemu crash when using scsi-block X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Deepa Srinivasan , Konrad Rzeszutek Wilk Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Starting qemu with the following arguments causes qemu to segfault: ... -device lsi,id=3Dlsi0 -drive file=3Discsi:<...>,format=3Draw,if=3Dnone,= node-name=3D iscsi1 -device scsi-block,bus=3Dlsi0.0,id=3D<...>,drive=3Discsi1 This patch fixes blk_aio_ioctl() so it does not pass stack addresses to blk_aio_ioctl_entry() which may be invoked after blk_aio_ioctl() returns. M= ore details about the bug follow. blk_aio_ioctl() invokes blk_aio_prwv() with blk_aio_ioctl_entry as the coroutine parameter. blk_aio_prwv() ultimately calls aio_co_enter(). When blk_aio_ioctl() is executed from within a coroutine context (e.g. iscsi_bh_cb()), aio_co_enter() adds the coroutine (blk_aio_ioctl_entry) to the current coroutine's wakeup queue. blk_aio_ioctl() then returns. When blk_aio_ioctl_entry() executes later, it accesses an invalid pointer: .... BlkRwCo *rwco =3D &acb->rwco; rwco->ret =3D blk_co_ioctl(rwco->blk, rwco->offset, rwco->qiov->iov[0].iov_base); <--- qiov is invalid he= re ... In the case when blk_aio_ioctl() is called from a non-coroutine context, blk_aio_ioctl_entry() executes immediately. But if bdrv_co_ioctl() calls qemu_coroutine_yield(), blk_aio_ioctl() will return. When the coroutine execution is complete, control returns to blk_aio_ioctl_entry() after the c= all to blk_co_ioctl(). There is no invalid reference after this point, but the function is still holding on to invalid pointers. The fix is to allocate memory for the QEMUIOVector and struct iovec as part= of the request struct which the IO buffer is part of. The memory for this stru= ct is guaranteed to be valid till the AIO is completed. Signed-off-by: Deepa Srinivasan Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Mark Kanda --- block/block-backend.c | 13 ++----------- hw/block/virtio-blk.c | 9 ++++++++- hw/scsi/scsi-disk.c | 10 +++++++++- hw/scsi/scsi-generic.c | 9 ++++++++- include/sysemu/block-backend.h | 2 +- 5 files changed, 28 insertions(+), 15 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index baef8e7..c275827 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1472,19 +1472,10 @@ static void blk_aio_ioctl_entry(void *opaque) blk_aio_complete(acb); } =20 -BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *= buf, +BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, QEMUIO= Vector *qiov, BlockCompletionFunc *cb, void *opaque) { - QEMUIOVector qiov; - struct iovec iov; - - iov =3D (struct iovec) { - .iov_base =3D buf, - .iov_len =3D 0, - }; - qemu_iovec_init_external(&qiov, &iov, 1); - - return blk_aio_prwv(blk, req, 0, &qiov, blk_aio_ioctl_entry, 0, cb, op= aque); + return blk_aio_prwv(blk, req, 0, qiov, blk_aio_ioctl_entry, 0, cb, opa= que); } =20 int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 05d1440..ed9f774 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -151,6 +151,8 @@ out: typedef struct { VirtIOBlockReq *req; struct sg_io_hdr hdr; + QEMUIOVector qiov; + struct iovec iov; } VirtIOBlockIoctlReq; =20 static void virtio_blk_ioctl_complete(void *opaque, int status) @@ -298,7 +300,12 @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *= req) ioctl_req->hdr.sbp =3D elem->in_sg[elem->in_num - 3].iov_base; ioctl_req->hdr.mx_sb_len =3D elem->in_sg[elem->in_num - 3].iov_len; =20 - acb =3D blk_aio_ioctl(blk->blk, SG_IO, &ioctl_req->hdr, + ioctl_req->iov.iov_base =3D &ioctl_req->hdr; + ioctl_req->iov.iov_len =3D 0; + + qemu_iovec_init_external(&ioctl_req->qiov, &ioctl_req->iov, 1); + + acb =3D blk_aio_ioctl(blk->blk, SG_IO, &ioctl_req->qiov, virtio_blk_ioctl_complete, ioctl_req); if (!acb) { g_free(ioctl_req); diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 1243117..7cbe18d 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2636,6 +2636,9 @@ typedef struct SCSIBlockReq { SCSIDiskReq req; sg_io_hdr_t io_header; =20 + QEMUIOVector qiov; + struct iovec iov; + /* Selected bytes of the original CDB, copied into our own CDB. */ uint8_t cmd, cdb1, group_number; =20 @@ -2722,7 +2725,12 @@ static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *= req, io_header->usr_ptr =3D r; io_header->flags |=3D SG_FLAG_DIRECT_IO; =20 - aiocb =3D blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header, cb, opaque= ); + req->iov.iov_base =3D io_header; + req->iov.iov_len =3D 0; + + qemu_iovec_init_external(&req->qiov, &req->iov, 1); + + aiocb =3D blk_aio_ioctl(s->qdev.conf.blk, SG_IO, &req->qiov, cb, opaqu= e); assert(aiocb !=3D NULL); return aiocb; } diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index bd0d9ff..856af7c 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -46,6 +46,8 @@ typedef struct SCSIGenericReq { int buflen; int len; sg_io_hdr_t io_header; + QEMUIOVector qiov; + struct iovec iov; } SCSIGenericReq; =20 static void scsi_generic_save_request(QEMUFile *f, SCSIRequest *req) @@ -135,7 +137,12 @@ static int execute_command(BlockBackend *blk, r->io_header.usr_ptr =3D r; r->io_header.flags |=3D SG_FLAG_DIRECT_IO; =20 - r->req.aiocb =3D blk_aio_ioctl(blk, SG_IO, &r->io_header, complete, r); + r->iov.iov_base =3D &r->io_header; + r->iov.iov_len =3D 0; + + qemu_iovec_init_external(&r->qiov, &r->iov, 1); + + r->req.aiocb =3D blk_aio_ioctl(blk, SG_IO, &r->qiov, complete, r); if (r->req.aiocb =3D=3D NULL) { return -EIO; } diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index c4e52a5..32f4486 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -151,7 +151,7 @@ void blk_aio_cancel(BlockAIOCB *acb); void blk_aio_cancel_async(BlockAIOCB *acb); int blk_co_ioctl(BlockBackend *blk, unsigned long int req, void *buf); int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf); -BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *= buf, +BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, QEMUIO= Vector *qiov, BlockCompletionFunc *cb, void *opaque); int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes); int blk_co_flush(BlockBackend *blk); --=20 2.7.4