From nobody Tue Feb 10 10:54:09 2026 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; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1517606461566143.59741433983652; Fri, 2 Feb 2018 13:21:01 -0800 (PST) Received: from localhost ([::1]:33275 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ehilP-0006tX-Cd for importer@patchew.org; Fri, 02 Feb 2018 16:20:51 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54266) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ehiho-0004Ke-OB for qemu-devel@nongnu.org; Fri, 02 Feb 2018 16:17:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ehihn-0000dK-Jg for qemu-devel@nongnu.org; Fri, 02 Feb 2018 16:17:08 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36222) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ehihi-0000Yq-LR; Fri, 02 Feb 2018 16:17:02 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EDE1F4E4C0; Fri, 2 Feb 2018 21:17:01 +0000 (UTC) Received: from localhost (ovpn-116-80.ams2.redhat.com [10.36.116.80]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5F4F0749C0; Fri, 2 Feb 2018 21:16:58 +0000 (UTC) From: Stefan Hajnoczi To: Date: Fri, 2 Feb 2018 22:16:28 +0100 Message-Id: <20180202211628.3661-4-stefanha@redhat.com> In-Reply-To: <20180202211628.3661-1-stefanha@redhat.com> References: <20180202211628.3661-1-stefanha@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Fri, 02 Feb 2018 21:17:02 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 3/3] block/iscsi: fix ioctl cancel use-after-free 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: Ronnie Sahlberg , qemu-block@nongnu.org, Peter Lieven , Stefan Hajnoczi , Felipe Franciosi , Paolo Bonzini 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" The ioctl request cancellation code assumes that requests do not complete once TASK ABORT has been sent to the iSCSI target. The request completion callback is unconditionally invoked when TASK ABORT finishes. Therefore the request completion callback is invoked twice if the request does happen to complete before TASK ABORT. Futhermore, iscsi_aio_cancel() does not increment the request's reference count, causing a use-after-free when TASK ABORT finishes after the request has already completed. The iscsilun->mutex protection is also missing in iscsi_aio_cancel(). This patch rewrites iscsi_aio_cancel() and iscsi_abort_task_cb() to avoid double completion, use-after-free, and to take iscsilun->mutex when needed. Reported-by: Felipe Franciosi Signed-off-by: Stefan Hajnoczi --- block/iscsi.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 1cfe1c647c..4566902d43 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -282,14 +282,19 @@ static void iscsi_co_init_iscsitask(IscsiLun *iscsilu= n, struct IscsiTask *iTask) }; } =20 +/* Called (via iscsi_service) with QemuMutex held. */ static void iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command= _data, void *private_data) { IscsiAIOCB *acb =3D private_data; =20 - acb->status =3D -ECANCELED; - iscsi_schedule_bh(acb); + /* Skip if the request already completed */ + if (acb->status =3D=3D -ECANCELED) { + iscsi_schedule_bh(acb); + } + + qemu_aio_unref(acb); /* acquired in iscsi_aio_cancel() */ } =20 static void @@ -298,14 +303,26 @@ iscsi_aio_cancel(BlockAIOCB *blockacb) IscsiAIOCB *acb =3D (IscsiAIOCB *)blockacb; IscsiLun *iscsilun =3D acb->iscsilun; =20 + qemu_mutex_lock(&iscsilun->mutex); + + /* If it was cancelled or completed already, our work is done here */ if (acb->status !=3D -EINPROGRESS) { + qemu_mutex_unlock(&iscsilun->mutex); return; } =20 + /* This can still be overwritten if the request completes */ + acb->status =3D -ECANCELED; + + qemu_aio_ref(acb); /* released in iscsi_abort_task_cb() */ + /* send a task mgmt call to the target to cancel the task on the targe= t */ - iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task, - iscsi_abort_task_cb, acb); + if (iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task, + iscsi_abort_task_cb, acb) < 0) { + qemu_aio_unref(acb); /* since iscsi_abort_task_cb() won't be calle= d */ + } =20 + qemu_mutex_unlock(&iscsilun->mutex); } =20 static const AIOCBInfo iscsi_aiocb_info =3D { --=20 2.14.3