dma-helpers: prevent dma_blk_cb() vs dma_aio_cancel() race
commitabfcd2760b3e70727bbc0792221b8b98a733dc32
authorStefan Hajnoczi <stefanha@redhat.com>
Tue, 21 Feb 2023 21:22:17 +0000 (21 16:22 -0500)
committerKevin Wolf <kwolf@redhat.com>
Thu, 23 Feb 2023 18:49:35 +0000 (23 19:49 +0100)
tree29cfea4f465984e135acbc4629dde02f959ada61
parent7b7fc3d0102dafe8eb44802493036a526e921a71
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>
hw/scsi/scsi-disk.c
softmmu/dma-helpers.c