Skip to content

Commit

Permalink
hw/nvme: remove copy bh scheduling
Browse files Browse the repository at this point in the history
Fix a potential use-after-free by removing the bottom half and enqueuing
the completion directly.

Fixes: 796d206 ("hw/nvme: reimplement the copy command to allow aio cancellation")
Reviewed-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
  • Loading branch information
birkelund committed Dec 1, 2022
1 parent 818b9b8 commit 83f56ac
Showing 1 changed file with 14 additions and 49 deletions.
63 changes: 14 additions & 49 deletions hw/nvme/ctrl.c
Expand Up @@ -2552,7 +2552,6 @@ typedef struct NvmeCopyAIOCB {
BlockAIOCB common;
BlockAIOCB *aiocb;
NvmeRequest *req;
QEMUBH *bh;
int ret;

void *ranges;
Expand Down Expand Up @@ -2590,9 +2589,8 @@ static const AIOCBInfo nvme_copy_aiocb_info = {
.cancel_async = nvme_copy_cancel,
};

static void nvme_copy_bh(void *opaque)
static void nvme_copy_done(NvmeCopyAIOCB *iocb)
{
NvmeCopyAIOCB *iocb = opaque;
NvmeRequest *req = iocb->req;
NvmeNamespace *ns = req->ns;
BlockAcctStats *stats = blk_get_stats(ns->blkconf.blk);
Expand All @@ -2604,9 +2602,6 @@ static void nvme_copy_bh(void *opaque)
qemu_iovec_destroy(&iocb->iov);
g_free(iocb->bounce);

qemu_bh_delete(iocb->bh);
iocb->bh = NULL;

if (iocb->ret < 0) {
block_acct_failed(stats, &iocb->acct.read);
block_acct_failed(stats, &iocb->acct.write);
Expand All @@ -2619,7 +2614,7 @@ static void nvme_copy_bh(void *opaque)
qemu_aio_unref(iocb);
}

static void nvme_copy_cb(void *opaque, int ret);
static void nvme_do_copy(NvmeCopyAIOCB *iocb);

static void nvme_copy_source_range_parse_format0(void *ranges, int idx,
uint64_t *slba, uint32_t *nlb,
Expand Down Expand Up @@ -2731,7 +2726,7 @@ static void nvme_copy_out_completed_cb(void *opaque, int ret)
iocb->idx++;
iocb->slba += nlb;
out:
nvme_copy_cb(iocb, iocb->ret);
nvme_do_copy(iocb);
}

static void nvme_copy_out_cb(void *opaque, int ret)
Expand All @@ -2743,18 +2738,10 @@ static void nvme_copy_out_cb(void *opaque, int ret)
size_t mlen;
uint8_t *mbounce;

if (ret < 0) {
iocb->ret = ret;
goto out;
} else if (iocb->ret < 0) {
if (ret < 0 || iocb->ret < 0 || !ns->lbaf.ms) {
goto out;
}

if (!ns->lbaf.ms) {
nvme_copy_out_completed_cb(iocb, 0);
return;
}

nvme_copy_source_range_parse(iocb->ranges, iocb->idx, iocb->format, NULL,
&nlb, NULL, NULL, NULL);

Expand All @@ -2771,7 +2758,7 @@ static void nvme_copy_out_cb(void *opaque, int ret)
return;

out:
nvme_copy_cb(iocb, ret);
nvme_copy_out_completed_cb(iocb, ret);
}

static void nvme_copy_in_completed_cb(void *opaque, int ret)
Expand Down Expand Up @@ -2865,15 +2852,9 @@ static void nvme_copy_in_completed_cb(void *opaque, int ret)

invalid:
req->status = status;
iocb->aiocb = NULL;
if (iocb->bh) {
qemu_bh_schedule(iocb->bh);
}

return;

iocb->ret = -1;
out:
nvme_copy_cb(iocb, ret);
nvme_do_copy(iocb);
}

static void nvme_copy_in_cb(void *opaque, int ret)
Expand All @@ -2884,18 +2865,10 @@ static void nvme_copy_in_cb(void *opaque, int ret)
uint64_t slba;
uint32_t nlb;

if (ret < 0) {
iocb->ret = ret;
goto out;
} else if (iocb->ret < 0) {
if (ret < 0 || iocb->ret < 0 || !ns->lbaf.ms) {
goto out;
}

if (!ns->lbaf.ms) {
nvme_copy_in_completed_cb(iocb, 0);
return;
}

nvme_copy_source_range_parse(iocb->ranges, iocb->idx, iocb->format, &slba,
&nlb, NULL, NULL, NULL);

Expand All @@ -2909,23 +2882,19 @@ static void nvme_copy_in_cb(void *opaque, int ret)
return;

out:
nvme_copy_cb(iocb, iocb->ret);
nvme_copy_in_completed_cb(iocb, ret);
}

static void nvme_copy_cb(void *opaque, int ret)
static void nvme_do_copy(NvmeCopyAIOCB *iocb)
{
NvmeCopyAIOCB *iocb = opaque;
NvmeRequest *req = iocb->req;
NvmeNamespace *ns = req->ns;
uint64_t slba;
uint32_t nlb;
size_t len;
uint16_t status;

if (ret < 0) {
iocb->ret = ret;
goto done;
} else if (iocb->ret < 0) {
if (iocb->ret < 0) {
goto done;
}

Expand Down Expand Up @@ -2972,14 +2941,11 @@ static void nvme_copy_cb(void *opaque, int ret)

invalid:
req->status = status;
iocb->ret = -1;
done:
iocb->aiocb = NULL;
if (iocb->bh) {
qemu_bh_schedule(iocb->bh);
}
nvme_copy_done(iocb);
}


static uint16_t nvme_copy(NvmeCtrl *n, NvmeRequest *req)
{
NvmeNamespace *ns = req->ns;
Expand Down Expand Up @@ -3049,7 +3015,6 @@ static uint16_t nvme_copy(NvmeCtrl *n, NvmeRequest *req)
}

iocb->req = req;
iocb->bh = qemu_bh_new(nvme_copy_bh, iocb);
iocb->ret = 0;
iocb->nr = nr;
iocb->idx = 0;
Expand All @@ -3066,7 +3031,7 @@ static uint16_t nvme_copy(NvmeCtrl *n, NvmeRequest *req)
BLOCK_ACCT_WRITE);

req->aiocb = &iocb->common;
nvme_copy_cb(iocb, 0);
nvme_do_copy(iocb);

return NVME_NO_COMPLETE;

Expand Down

0 comments on commit 83f56ac

Please sign in to comment.