mirror of https://github.com/xemu-project/xemu.git
hw/nvme: remove copy bh scheduling
Fix a potential use-after-free by removing the bottom half and enqueuing
the completion directly.
Fixes: 796d20681d
("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>
This commit is contained in:
parent
818b9b8f5e
commit
83f56ac321
|
@ -2552,7 +2552,6 @@ typedef struct NvmeCopyAIOCB {
|
||||||
BlockAIOCB common;
|
BlockAIOCB common;
|
||||||
BlockAIOCB *aiocb;
|
BlockAIOCB *aiocb;
|
||||||
NvmeRequest *req;
|
NvmeRequest *req;
|
||||||
QEMUBH *bh;
|
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
void *ranges;
|
void *ranges;
|
||||||
|
@ -2590,9 +2589,8 @@ static const AIOCBInfo nvme_copy_aiocb_info = {
|
||||||
.cancel_async = nvme_copy_cancel,
|
.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;
|
NvmeRequest *req = iocb->req;
|
||||||
NvmeNamespace *ns = req->ns;
|
NvmeNamespace *ns = req->ns;
|
||||||
BlockAcctStats *stats = blk_get_stats(ns->blkconf.blk);
|
BlockAcctStats *stats = blk_get_stats(ns->blkconf.blk);
|
||||||
|
@ -2604,9 +2602,6 @@ static void nvme_copy_bh(void *opaque)
|
||||||
qemu_iovec_destroy(&iocb->iov);
|
qemu_iovec_destroy(&iocb->iov);
|
||||||
g_free(iocb->bounce);
|
g_free(iocb->bounce);
|
||||||
|
|
||||||
qemu_bh_delete(iocb->bh);
|
|
||||||
iocb->bh = NULL;
|
|
||||||
|
|
||||||
if (iocb->ret < 0) {
|
if (iocb->ret < 0) {
|
||||||
block_acct_failed(stats, &iocb->acct.read);
|
block_acct_failed(stats, &iocb->acct.read);
|
||||||
block_acct_failed(stats, &iocb->acct.write);
|
block_acct_failed(stats, &iocb->acct.write);
|
||||||
|
@ -2619,7 +2614,7 @@ static void nvme_copy_bh(void *opaque)
|
||||||
qemu_aio_unref(iocb);
|
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,
|
static void nvme_copy_source_range_parse_format0(void *ranges, int idx,
|
||||||
uint64_t *slba, uint32_t *nlb,
|
uint64_t *slba, uint32_t *nlb,
|
||||||
|
@ -2731,7 +2726,7 @@ static void nvme_copy_out_completed_cb(void *opaque, int ret)
|
||||||
iocb->idx++;
|
iocb->idx++;
|
||||||
iocb->slba += nlb;
|
iocb->slba += nlb;
|
||||||
out:
|
out:
|
||||||
nvme_copy_cb(iocb, iocb->ret);
|
nvme_do_copy(iocb);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void nvme_copy_out_cb(void *opaque, int ret)
|
static void nvme_copy_out_cb(void *opaque, int ret)
|
||||||
|
@ -2743,16 +2738,8 @@ static void nvme_copy_out_cb(void *opaque, int ret)
|
||||||
size_t mlen;
|
size_t mlen;
|
||||||
uint8_t *mbounce;
|
uint8_t *mbounce;
|
||||||
|
|
||||||
if (ret < 0) {
|
if (ret < 0 || iocb->ret < 0 || !ns->lbaf.ms) {
|
||||||
iocb->ret = ret;
|
|
||||||
goto out;
|
goto out;
|
||||||
} else if (iocb->ret < 0) {
|
|
||||||
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,
|
nvme_copy_source_range_parse(iocb->ranges, iocb->idx, iocb->format, NULL,
|
||||||
|
@ -2771,7 +2758,7 @@ static void nvme_copy_out_cb(void *opaque, int ret)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
out:
|
out:
|
||||||
nvme_copy_cb(iocb, ret);
|
nvme_copy_out_completed_cb(iocb, ret);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void nvme_copy_in_completed_cb(void *opaque, int ret)
|
static void nvme_copy_in_completed_cb(void *opaque, int ret)
|
||||||
|
@ -2865,15 +2852,9 @@ static void nvme_copy_in_completed_cb(void *opaque, int ret)
|
||||||
|
|
||||||
invalid:
|
invalid:
|
||||||
req->status = status;
|
req->status = status;
|
||||||
iocb->aiocb = NULL;
|
iocb->ret = -1;
|
||||||
if (iocb->bh) {
|
|
||||||
qemu_bh_schedule(iocb->bh);
|
|
||||||
}
|
|
||||||
|
|
||||||
return;
|
|
||||||
|
|
||||||
out:
|
out:
|
||||||
nvme_copy_cb(iocb, ret);
|
nvme_do_copy(iocb);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void nvme_copy_in_cb(void *opaque, int ret)
|
static void nvme_copy_in_cb(void *opaque, int ret)
|
||||||
|
@ -2884,16 +2865,8 @@ static void nvme_copy_in_cb(void *opaque, int ret)
|
||||||
uint64_t slba;
|
uint64_t slba;
|
||||||
uint32_t nlb;
|
uint32_t nlb;
|
||||||
|
|
||||||
if (ret < 0) {
|
if (ret < 0 || iocb->ret < 0 || !ns->lbaf.ms) {
|
||||||
iocb->ret = ret;
|
|
||||||
goto out;
|
goto out;
|
||||||
} else if (iocb->ret < 0) {
|
|
||||||
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,
|
nvme_copy_source_range_parse(iocb->ranges, iocb->idx, iocb->format, &slba,
|
||||||
|
@ -2909,12 +2882,11 @@ static void nvme_copy_in_cb(void *opaque, int ret)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
out:
|
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;
|
NvmeRequest *req = iocb->req;
|
||||||
NvmeNamespace *ns = req->ns;
|
NvmeNamespace *ns = req->ns;
|
||||||
uint64_t slba;
|
uint64_t slba;
|
||||||
|
@ -2922,10 +2894,7 @@ static void nvme_copy_cb(void *opaque, int ret)
|
||||||
size_t len;
|
size_t len;
|
||||||
uint16_t status;
|
uint16_t status;
|
||||||
|
|
||||||
if (ret < 0) {
|
if (iocb->ret < 0) {
|
||||||
iocb->ret = ret;
|
|
||||||
goto done;
|
|
||||||
} else if (iocb->ret < 0) {
|
|
||||||
goto done;
|
goto done;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2972,13 +2941,10 @@ static void nvme_copy_cb(void *opaque, int ret)
|
||||||
|
|
||||||
invalid:
|
invalid:
|
||||||
req->status = status;
|
req->status = status;
|
||||||
|
iocb->ret = -1;
|
||||||
done:
|
done:
|
||||||
iocb->aiocb = NULL;
|
nvme_copy_done(iocb);
|
||||||
if (iocb->bh) {
|
|
||||||
qemu_bh_schedule(iocb->bh);
|
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
static uint16_t nvme_copy(NvmeCtrl *n, NvmeRequest *req)
|
static uint16_t nvme_copy(NvmeCtrl *n, NvmeRequest *req)
|
||||||
{
|
{
|
||||||
|
@ -3049,7 +3015,6 @@ static uint16_t nvme_copy(NvmeCtrl *n, NvmeRequest *req)
|
||||||
}
|
}
|
||||||
|
|
||||||
iocb->req = req;
|
iocb->req = req;
|
||||||
iocb->bh = qemu_bh_new(nvme_copy_bh, iocb);
|
|
||||||
iocb->ret = 0;
|
iocb->ret = 0;
|
||||||
iocb->nr = nr;
|
iocb->nr = nr;
|
||||||
iocb->idx = 0;
|
iocb->idx = 0;
|
||||||
|
@ -3066,7 +3031,7 @@ static uint16_t nvme_copy(NvmeCtrl *n, NvmeRequest *req)
|
||||||
BLOCK_ACCT_WRITE);
|
BLOCK_ACCT_WRITE);
|
||||||
|
|
||||||
req->aiocb = &iocb->common;
|
req->aiocb = &iocb->common;
|
||||||
nvme_copy_cb(iocb, 0);
|
nvme_do_copy(iocb);
|
||||||
|
|
||||||
return NVME_NO_COMPLETE;
|
return NVME_NO_COMPLETE;
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue