Am 10.02.2023 um 15:32 hat Stefan Hajnoczi geschrieben: > If requests are being processed in the IOThread when a SCSIDevice is > unplugged, scsi_device_purge_requests() -> scsi_req_cancel_async() races > with I/O completion callbacks. Both threads load and store req->aiocb. > This can lead to assert(r->req.aiocb == NULL) failures and undefined > behavior. > > Protect r->req.aiocb with the AioContext lock to prevent the race. > > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
I tried to check that all accesses of .aiocb are actually protected by the AioContext lock. I stopped at scsi_read_data(), which asserts that it is non-NULL, but it is only called as a SCSIReqOps function. I couldn't find any information on what the locking rules are with SCSIReqOps functions and didn't feel like reverse engineering scsi-bus etc. without just asking first. The same question applies to: - scsi_read_data - scsi_write_data - scsi_disk_emulate_write_data - scsi_disk_emulate_command Since these are not callbacks scheduled in the AioContext by scsi-disk itself, I expect that they are indeed covered. The acquire/release pair in virtio_scsi_handle_cmd() might actually indirectly cover all of them, but I haven't checked that. Either way, the changes that you're making look good: Reviewed-by: Kevin Wolf <kw...@redhat.com>
