[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/3] scsi: protect req->aiocb with AioContext lock
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v2 1/3] scsi: protect req->aiocb with AioContext lock |
Date: |
Thu, 16 Feb 2023 13:34:30 +0100 |
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 <stefanha@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 <kwolf@redhat.com>