From nobody Fri Nov 7 00:48:59 2025 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; dkim=fail; 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 (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1544628520321180.75591031383465; Wed, 12 Dec 2018 07:28:40 -0800 (PST) Received: from localhost ([::1]:45584 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gX6RD-0004pI-8Q for importer@patchew.org; Wed, 12 Dec 2018 10:28:39 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52088) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gX6MR-0000hE-3l for qemu-devel@nongnu.org; Wed, 12 Dec 2018 10:23:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gX6MO-00006n-F1 for qemu-devel@nongnu.org; Wed, 12 Dec 2018 10:23:42 -0500 Received: from mail-wm1-x330.google.com ([2a00:1450:4864:20::330]:37659) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gX6MK-0008VB-O6 for qemu-devel@nongnu.org; Wed, 12 Dec 2018 10:23:38 -0500 Received: by mail-wm1-x330.google.com with SMTP id g67so6365305wmd.2 for ; Wed, 12 Dec 2018 07:23:35 -0800 (PST) Received: from 640k.lan ([93.56.166.5]) by smtp.gmail.com with ESMTPSA id u10sm15878859wrr.33.2018.12.12.07.23.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 12 Dec 2018 07:23:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=jq9RYK9Nl/q9VaGrDtTKXipkoxjHVEAmE049EAw6qrY=; b=JUUJOoj/7dI+O8E7cbxIVOmoccka6hicladNm7Ua/Vk3RyQ1nV0/WcfYeuA+839NV7 sdxzBcJsqR0GaDnRzUqZT/eeWujON3tuc9LiXFmXBo7bg+nPgm3cKUhKNmCafpIiEwpJ XMrGF7uGByZSDWYHmIZg5vtybCgpQqNrCbf5FbyGL2LyWTzWxSMeL4mndPPLGRVFSJqU Cn+Rll5aRRTwqvEup8RUJWgXAXC08Z8gS1OyvXp/wYLun/0VIMOwszOv/IeQp9wAWEUR YJ2D6DLJ4NszwBikjqFraykPMu+6kSrDH8TpztHDc0dFm0C2DguQC92s1Con33Erpxxz YS3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=jq9RYK9Nl/q9VaGrDtTKXipkoxjHVEAmE049EAw6qrY=; b=Dz94pyTh2PWWbc8y72XHbfcQnBFSApgNdGh2c7mF/sDe//NAz9XdSVRncc5oEd5S3L nevF6G5sz4xFMeJvnyX7kSVtIPy8o+SVw5GOFgQjMtkuxz30/bq9CKfAelBVqETf87xj aArz2UWI2ONjKNlpXr+ioHE/woLTpbga2XuV0P6jqOGC34+Vv2myMl0PPoLq8dxS5J7j 8LIPhP9pNNz5KJW7fBVfoZYkoLxZ7AggnYBcTOJeWQqvSfKOSCDcUeceyQw37XDw9Q6D wq3wnr+AbnU24uqay6EE3FB/QJn91rp1JIu/Lt9Lg3GfwiDhXmlYwNZt0Kj7gmYOnwEJ zJxg== X-Gm-Message-State: AA+aEWZyzwmtR7CsBf5q133zxkizUtbZVEywT4zcl0LgVM81gYDGqcuI 1C92dm1rUhFCD7rpOCAYJjxtR8Br X-Google-Smtp-Source: AFSGD/WhmmodNxts1y3cclbC/btF0vrSwDlk8rqEsF9NeVP8vAkLWOq6i2UUQqr6zg3EktmWS44guQ== X-Received: by 2002:a7b:cc86:: with SMTP id p6mr711179wma.19.1544628214194; Wed, 12 Dec 2018 07:23:34 -0800 (PST) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Wed, 12 Dec 2018 16:22:35 +0100 Message-Id: <1544628195-37728-15-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1544628195-37728-1-git-send-email-pbonzini@redhat.com> References: <1544628195-37728-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::330 Subject: [Qemu-devel] [PULL 14/54] 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: Stefan Hajnoczi Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Stefan Hajnoczi iscsi_aio_cancel() does not increment the request's reference count, causing a use-after-free when ABORT TASK finishes after the request has already completed. There are some additional issues with iscsi_aio_cancel(): 1. Several ABORT TASKs may be sent for the same task if iscsi_aio_cancel() is invoked multiple times. It's better to avoid this just in case the command identifier is reused. 2. The iscsilun->mutex protection is missing in iscsi_aio_cancel(). Reported-by: Felipe Franciosi Signed-off-by: Stefan Hajnoczi Message-Id: <20180203061621.7033-4-stefanha@redhat.com> Reviewed-by: Felipe Franciosi Tested-by: Sreejith Mohanan Signed-off-by: Paolo Bonzini --- block/iscsi.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 1924a2b..abb872d 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -124,6 +124,7 @@ typedef struct IscsiAIOCB { #ifdef __linux__ sg_io_hdr_t *ioh; #endif + bool cancelled; } IscsiAIOCB; =20 /* libiscsi uses time_t so its enough to process events every second */ @@ -287,6 +288,7 @@ static void iscsi_co_init_iscsitask(IscsiLun *iscsilun,= 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) @@ -295,6 +297,7 @@ iscsi_abort_task_cb(struct iscsi_context *iscsi, int st= atus, void *command_data, =20 acb->status =3D -ECANCELED; iscsi_schedule_bh(acb); + qemu_aio_unref(acb); /* acquired in iscsi_aio_cancel() */ } =20 static void @@ -303,14 +306,25 @@ iscsi_aio_cancel(BlockAIOCB *blockacb) IscsiAIOCB *acb =3D (IscsiAIOCB *)blockacb; IscsiLun *iscsilun =3D acb->iscsilun; =20 - if (acb->status !=3D -EINPROGRESS) { + qemu_mutex_lock(&iscsilun->mutex); + + /* If it was cancelled or completed already, our work is done here */ + if (acb->cancelled || acb->status !=3D -EINPROGRESS) { + qemu_mutex_unlock(&iscsilun->mutex); return; } =20 + acb->cancelled =3D true; + + 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 { @@ -1008,6 +1022,7 @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *= bs, acb->bh =3D NULL; acb->status =3D -EINPROGRESS; acb->ioh =3D buf; + acb->cancelled =3D false; =20 if (req !=3D SG_IO) { iscsi_ioctl_handle_emulated(acb, req, buf); --=20 1.8.3.1