> [Severity: Medium]
> If a device is removed while IOMMU requests are queued but not yet synced,
> does this early return bypass __viommu_sync_req() where those pending
> requests would normally be freed?
> When setting viommu->removed to true during teardown, do we also need to
> iterate over the viommu->requests list and free any abandoned
> viommu_request objects?
Yes, they are left behind, and it is a real leak. They cannot be freed at
that point, though: the virtqueue still owns them - it holds them as the
buffer data and has their scatterlists mapped - and the device may still be
using them until it is reset a few lines later.
The cleanup belongs after the reset and before the queues are deleted,
where virtqueue_detach_unused_buf() releases both the object and its DMA
mapping, as other drivers do at removal - and that is what the v2 I just
sent does:
vq = viommu->vqs[VIOMMU_REQUEST_VQ];
while ((req = virtqueue_detach_unused_buf(vq))) {
list_del(&req->list);
kfree(req);
}
vdev->config->del_vqs(vdev);
The baseline did not clean up either, but worse: with no removal state it
would call virtqueue_get_buf() on the already-deleted virtqueues, i.e.
use-after-free, rather than leak. The flag turns that into "queueing and
draining stop"; the loop above closes the loop on what is left.
Thanks,
Anlai
