From patchwork Mon Dec 5 09:51:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juan Quintela X-Patchwork-Id: 13064331 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 739E8C4321E for ; Mon, 5 Dec 2022 10:01:38 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p28C4-0002vS-88; Mon, 05 Dec 2022 04:55:24 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p28C0-0002u2-Ua for qemu-devel@nongnu.org; Mon, 05 Dec 2022 04:55:21 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p28Bz-0007I8-5t for qemu-devel@nongnu.org; Mon, 05 Dec 2022 04:55:20 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1670234118; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=e3XgTZKxlY8kzFFBQsUmpC/wSN3ufoA5pXR8Z5sxu4s=; b=AQpBSVMPcOBmAfQMz13c+/I3n8bKi1ANNLCKrOiRbJBqNrOBHWEcBGiBZxXu5/KySLI8kb lAxF96dr15z9jjfVeBz3YHQTNmBTtK1TEyWHwXX2fzq9ReByeUbOj2u9QJ2GG1DL/nkCqz 7zYZ/Fls0CZ1aS9dkjFHH7uxWuOy//k= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-263-EvfnM0ieOlWUXJVVxy2KuA-1; Mon, 05 Dec 2022 04:55:17 -0500 X-MC-Unique: EvfnM0ieOlWUXJVVxy2KuA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 56D7E1C0014B; Mon, 5 Dec 2022 09:55:16 +0000 (UTC) Received: from secure.mitica (unknown [10.39.194.135]) by smtp.corp.redhat.com (Postfix) with ESMTP id D99072166B29; Mon, 5 Dec 2022 09:55:08 +0000 (UTC) From: Juan Quintela To: qemu-devel@nongnu.org Cc: Fam Zheng , Thomas Huth , Viresh Kumar , Kevin Wolf , Mathieu Poirier , Laurent Vivier , Eric Blake , Richard Henderson , Raphael Norwitz , Stefan Hajnoczi , Juan Quintela , virtio-fs@redhat.com, =?utf-8?q?Alex_Benn=C3=A9e?= , Christian Borntraeger , "Gonglei (Arei)" , qemu-block@nongnu.org, Xiaojuan Yang , Thomas Huth , Ilya Leoshkevich , Eduardo Habkost , Gerd Hoffmann , "Dr. David Alan Gilbert" , Alex Williamson , Eric Farman , Halil Pasic , Peter Maydell , Vladimir Sementsov-Ogievskiy , Jason Wang , Laurent Vivier , Song Gao , qemu-s390x@nongnu.org, Pavel Dovgalyuk , Klaus Jensen , John Snow , Michael Tokarev , qemu-arm@nongnu.org, Paolo Bonzini , "Michael S. Tsirkin" , Keith Busch , David Hildenbrand , qemu-trivial@nongnu.org, Hanna Reitz , Klaus Jensen Subject: [PATCH v2 20/51] hw/nvme: fix aio cancel in dsm Date: Mon, 5 Dec 2022 10:51:57 +0100 Message-Id: <20221205095228.1314-21-quintela@redhat.com> In-Reply-To: <20221205095228.1314-1-quintela@redhat.com> References: <20221205095228.1314-1-quintela@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 Received-SPF: pass client-ip=170.10.129.124; envelope-from=quintela@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org From: Klaus Jensen When the DSM operation is cancelled asynchronously, we set iocb->ret to -ECANCELED. However, the callback function only checks the return value of the completed aio, which may have completed succesfully prior to the cancellation and thus the callback ends up continuing the dsm operation instead of bailing out. Fix this. Secondly, fix a potential use-after-free by removing the bottom half and enqueuing the completion directly. Fixes: d7d1474fd85d ("hw/nvme: reimplement dsm to allow cancellation") Reviewed-by: Keith Busch Signed-off-by: Klaus Jensen --- hw/nvme/ctrl.c | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index bf4abf73f7..e847b89461 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -2329,7 +2329,6 @@ typedef struct NvmeDSMAIOCB { BlockAIOCB common; BlockAIOCB *aiocb; NvmeRequest *req; - QEMUBH *bh; int ret; NvmeDsmRange *range; @@ -2351,7 +2350,7 @@ static void nvme_dsm_cancel(BlockAIOCB *aiocb) } else { /* * We only reach this if nvme_dsm_cancel() has already been called or - * the command ran to completion and nvme_dsm_bh is scheduled to run. + * the command ran to completion. */ assert(iocb->idx == iocb->nr); } @@ -2362,17 +2361,6 @@ static const AIOCBInfo nvme_dsm_aiocb_info = { .cancel_async = nvme_dsm_cancel, }; -static void nvme_dsm_bh(void *opaque) -{ - NvmeDSMAIOCB *iocb = opaque; - - iocb->common.cb(iocb->common.opaque, iocb->ret); - - qemu_bh_delete(iocb->bh); - iocb->bh = NULL; - qemu_aio_unref(iocb); -} - static void nvme_dsm_cb(void *opaque, int ret); static void nvme_dsm_md_cb(void *opaque, int ret) @@ -2384,16 +2372,10 @@ static void nvme_dsm_md_cb(void *opaque, int ret) uint64_t slba; uint32_t nlb; - if (ret < 0) { - iocb->ret = ret; + if (ret < 0 || iocb->ret < 0 || !ns->lbaf.ms) { goto done; } - if (!ns->lbaf.ms) { - nvme_dsm_cb(iocb, 0); - return; - } - range = &iocb->range[iocb->idx - 1]; slba = le64_to_cpu(range->slba); nlb = le32_to_cpu(range->nlb); @@ -2406,7 +2388,6 @@ static void nvme_dsm_md_cb(void *opaque, int ret) ret = nvme_block_status_all(ns, slba, nlb, BDRV_BLOCK_ZERO); if (ret) { if (ret < 0) { - iocb->ret = ret; goto done; } @@ -2420,8 +2401,7 @@ static void nvme_dsm_md_cb(void *opaque, int ret) return; done: - iocb->aiocb = NULL; - qemu_bh_schedule(iocb->bh); + nvme_dsm_cb(iocb, ret); } static void nvme_dsm_cb(void *opaque, int ret) @@ -2434,7 +2414,9 @@ static void nvme_dsm_cb(void *opaque, int ret) uint64_t slba; uint32_t nlb; - if (ret < 0) { + if (iocb->ret < 0) { + goto done; + } else if (ret < 0) { iocb->ret = ret; goto done; } @@ -2468,7 +2450,8 @@ next: done: iocb->aiocb = NULL; - qemu_bh_schedule(iocb->bh); + iocb->common.cb(iocb->common.opaque, iocb->ret); + qemu_aio_unref(iocb); } static uint16_t nvme_dsm(NvmeCtrl *n, NvmeRequest *req) @@ -2486,7 +2469,6 @@ static uint16_t nvme_dsm(NvmeCtrl *n, NvmeRequest *req) nvme_misc_cb, req); iocb->req = req; - iocb->bh = qemu_bh_new(nvme_dsm_bh, iocb); iocb->ret = 0; iocb->range = g_new(NvmeDsmRange, nr); iocb->nr = nr;