diff options
author | Klaus Jensen | 2022-11-10 07:59:47 +0100 |
---|---|---|
committer | Klaus Jensen | 2022-12-01 08:44:56 +0100 |
commit | 36a251c3468f34a2486dd49836e397534a1bb189 (patch) | |
tree | bf913084098b567e45ee44e6a6272eea22b968d3 | |
parent | hw/nvme: fix aio cancel in flush (diff) | |
download | qemu-36a251c3468f34a2486dd49836e397534a1bb189.tar.gz qemu-36a251c3468f34a2486dd49836e397534a1bb189.tar.xz qemu-36a251c3468f34a2486dd49836e397534a1bb189.zip |
hw/nvme: fix aio cancel in zone reset
If the zone reset operation is cancelled but the block unmap operation
completes normally, the callback will continue resetting the next zone
since it neglects to check iocb->ret which will have been set to
-ECANCELED. Make sure that this is checked and bail out if an error is
present.
Secondly, fix a potential use-after-free by removing the bottom half and
enqueuing the completion directly.
Fixes: 63d96e4ffd71 ("hw/nvme: reimplement zone reset to allow cancellation")
Reviewed-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
-rw-r--r-- | hw/nvme/ctrl.c | 36 |
1 files changed, 11 insertions, 25 deletions
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index fede5af6af..bf4abf73f7 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -3712,7 +3712,6 @@ typedef struct NvmeZoneResetAIOCB { BlockAIOCB common; BlockAIOCB *aiocb; NvmeRequest *req; - QEMUBH *bh; int ret; bool all; @@ -3741,17 +3740,6 @@ static const AIOCBInfo nvme_zone_reset_aiocb_info = { .cancel_async = nvme_zone_reset_cancel, }; -static void nvme_zone_reset_bh(void *opaque) -{ - NvmeZoneResetAIOCB *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_zone_reset_cb(void *opaque, int ret); static void nvme_zone_reset_epilogue_cb(void *opaque, int ret) @@ -3762,14 +3750,8 @@ static void nvme_zone_reset_epilogue_cb(void *opaque, int ret) int64_t moff; int count; - if (ret < 0) { - nvme_zone_reset_cb(iocb, ret); - return; - } - - if (!ns->lbaf.ms) { - nvme_zone_reset_cb(iocb, 0); - return; + if (ret < 0 || iocb->ret < 0 || !ns->lbaf.ms) { + goto out; } moff = nvme_moff(ns, iocb->zone->d.zslba); @@ -3779,6 +3761,9 @@ static void nvme_zone_reset_epilogue_cb(void *opaque, int ret) BDRV_REQ_MAY_UNMAP, nvme_zone_reset_cb, iocb); return; + +out: + nvme_zone_reset_cb(iocb, ret); } static void nvme_zone_reset_cb(void *opaque, int ret) @@ -3787,7 +3772,9 @@ static void nvme_zone_reset_cb(void *opaque, int ret) NvmeRequest *req = iocb->req; NvmeNamespace *ns = req->ns; - if (ret < 0) { + if (iocb->ret < 0) { + goto done; + } else if (ret < 0) { iocb->ret = ret; goto done; } @@ -3835,9 +3822,9 @@ static void nvme_zone_reset_cb(void *opaque, int ret) done: iocb->aiocb = NULL; - if (iocb->bh) { - qemu_bh_schedule(iocb->bh); - } + + iocb->common.cb(iocb->common.opaque, iocb->ret); + qemu_aio_unref(iocb); } static uint16_t nvme_zone_mgmt_send_zrwa_flush(NvmeCtrl *n, NvmeZone *zone, @@ -3942,7 +3929,6 @@ static uint16_t nvme_zone_mgmt_send(NvmeCtrl *n, NvmeRequest *req) nvme_misc_cb, req); iocb->req = req; - iocb->bh = qemu_bh_new(nvme_zone_reset_bh, iocb); iocb->ret = 0; iocb->all = all; iocb->idx = zone_idx; |