From patchwork Wed Aug 15 13:56:41 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 177667 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id E73C82C007F for ; Wed, 15 Aug 2012 23:59:19 +1000 (EST) Received: from localhost ([::1]:42073 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T1e6m-0000Qd-3o for incoming@patchwork.ozlabs.org; Wed, 15 Aug 2012 09:58:04 -0400 Received: from eggs.gnu.org ([208.118.235.92]:39336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T1e5o-0006co-9c for qemu-devel@nongnu.org; Wed, 15 Aug 2012 09:57:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T1e5m-0000Qu-KO for qemu-devel@nongnu.org; Wed, 15 Aug 2012 09:57:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14681) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T1e5m-0000Qm-CD for qemu-devel@nongnu.org; Wed, 15 Aug 2012 09:57:02 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q7FDv0ZQ030636 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 15 Aug 2012 09:57:01 -0400 Received: from dhcp-5-188.str.redhat.com (dhcp-5-175.str.redhat.com [10.32.5.175]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q7FDunab031735; Wed, 15 Aug 2012 09:56:59 -0400 From: Kevin Wolf To: anthony@codemonkey.ws Date: Wed, 15 Aug 2012 15:56:41 +0200 Message-Id: <1345039003-19186-9-git-send-email-kwolf@redhat.com> In-Reply-To: <1345039003-19186-1-git-send-email-kwolf@redhat.com> References: <1345039003-19186-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, qemu-devel@nongnu.org Subject: [Qemu-devel] [PATCH 08/10] iscsi: Fix NULL dereferences / races between task completion and abort 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 From: Stefan Priebe Signed-off-by: Stefan Priebe Acked-by: Ronnie Sahlberg Signed-off-by: Kevin Wolf --- block/iscsi.c | 55 +++++++++++++++++++++++-------------------------------- 1 files changed, 23 insertions(+), 32 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 219f927..bb9cf82 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -76,6 +76,10 @@ static void iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data, void *private_data) { + IscsiAIOCB *acb = (IscsiAIOCB *)private_data; + + scsi_free_scsi_task(acb->task); + acb->task = NULL; } static void @@ -84,15 +88,15 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb) IscsiAIOCB *acb = (IscsiAIOCB *)blockacb; IscsiLun *iscsilun = acb->iscsilun; - acb->common.cb(acb->common.opaque, -ECANCELED); acb->canceled = 1; - /* send a task mgmt call to the target to cancel the task on the target */ - iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task, - iscsi_abort_task_cb, NULL); + acb->common.cb(acb->common.opaque, -ECANCELED); - /* then also cancel the task locally in libiscsi */ - iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task); + /* send a task mgmt call to the target to cancel the task on the target + * this also cancels the task in libiscsi + */ + iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task, + iscsi_abort_task_cb, &acb); } static AIOPool iscsi_aio_pool = { @@ -179,11 +183,18 @@ iscsi_readv_writev_bh_cb(void *p) qemu_bh_delete(acb->bh); - if (acb->canceled == 0) { + if (!acb->canceled) { acb->common.cb(acb->common.opaque, acb->status); } qemu_aio_release(acb); + + if (acb->canceled) { + return; + } + + scsi_free_scsi_task(acb->task); + acb->task = NULL; } @@ -197,10 +208,8 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status, g_free(acb->buf); - if (acb->canceled != 0) { + if (acb->canceled) { qemu_aio_release(acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; return; } @@ -212,8 +221,6 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status, } iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; } static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun) @@ -298,10 +305,8 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int status, trace_iscsi_aio_read16_cb(iscsi, status, acb, acb->canceled); - if (acb->canceled != 0) { + if (acb->canceled) { qemu_aio_release(acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; return; } @@ -313,8 +318,6 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int status, } iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; } static BlockDriverAIOCB * @@ -414,10 +417,8 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int status, { IscsiAIOCB *acb = opaque; - if (acb->canceled != 0) { + if (acb->canceled) { qemu_aio_release(acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; return; } @@ -429,8 +430,6 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int status, } iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; } static BlockDriverAIOCB * @@ -468,10 +467,8 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status, { IscsiAIOCB *acb = opaque; - if (acb->canceled != 0) { + if (acb->canceled) { qemu_aio_release(acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; return; } @@ -483,8 +480,6 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status, } iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; } static BlockDriverAIOCB * @@ -528,10 +523,8 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, { IscsiAIOCB *acb = opaque; - if (acb->canceled != 0) { + if (acb->canceled) { qemu_aio_release(acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; return; } @@ -560,8 +553,6 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, } iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb); - scsi_free_scsi_task(acb->task); - acb->task = NULL; } static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,