aboutsummaryrefslogtreecommitdiff
path: root/hw/scsi/scsi-disk.c
diff options
context:
space:
mode:
authorStefan Hajnoczi <stefanha@redhat.com>2023-02-21 16:22:17 -0500
committerKevin Wolf <kwolf@redhat.com>2023-02-23 19:49:35 +0100
commitabfcd2760b3e70727bbc0792221b8b98a733dc32 (patch)
tree29cfea4f465984e135acbc4629dde02f959ada61 /hw/scsi/scsi-disk.c
parent7b7fc3d0102dafe8eb44802493036a526e921a71 (diff)
downloadqemu-abfcd2760b3e70727bbc0792221b8b98a733dc32.zip
qemu-abfcd2760b3e70727bbc0792221b8b98a733dc32.tar.gz
qemu-abfcd2760b3e70727bbc0792221b8b98a733dc32.tar.bz2
dma-helpers: prevent dma_blk_cb() vs dma_aio_cancel() race
dma_blk_cb() only takes the AioContext lock around ->io_func(). That means the rest of dma_blk_cb() is not protected. In particular, the DMAAIOCB field accesses happen outside the lock. There is a race when the main loop thread holds the AioContext lock and invokes scsi_device_purge_requests() -> bdrv_aio_cancel() -> dma_aio_cancel() while an IOThread executes dma_blk_cb(). The dbs->acb field determines how cancellation proceeds. If dma_aio_cancel() sees dbs->acb == NULL while dma_blk_cb() is still running, the request can be completed twice (-ECANCELED and the actual return value). The following assertion can occur with virtio-scsi when an IOThread is used: ../hw/scsi/scsi-disk.c:368: scsi_dma_complete: Assertion `r->req.aiocb != NULL' failed. Fix the race by holding the AioContext across dma_blk_cb(). Now dma_aio_cancel() under the AioContext lock will not see inconsistent/intermediate states. Cc: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20230221212218.1378734-3-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Diffstat (limited to 'hw/scsi/scsi-disk.c')
-rw-r--r--hw/scsi/scsi-disk.c4
1 files changed, 1 insertions, 3 deletions
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 115584f..97c9b1c 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -354,13 +354,12 @@ done:
scsi_req_unref(&r->req);
}
+/* Called with AioContext lock held */
static void scsi_dma_complete(void *opaque, int ret)
{
SCSIDiskReq *r = (SCSIDiskReq *)opaque;
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
- aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
-
assert(r->req.aiocb != NULL);
r->req.aiocb = NULL;
@@ -370,7 +369,6 @@ static void scsi_dma_complete(void *opaque, int ret)
block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
}
scsi_dma_complete_noio(r, ret);
- aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
}
static void scsi_read_complete_noio(SCSIDiskReq *r, int ret)