Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] iommu/virtio: memory leak of queued requests on device removal --
commit db185ffd41ba01e7309f7ee2d8c108a5d39c6450 Author: Anlai Lu <[email protected]> iommu/virtio: batch the mapping replay on domain attach This commit optimizes the domain attach process by queueing all mapping requests and waiting for them once, rather than sending them synchronously one by one. This significantly reduces the IRQ-off window and attach time when dealing with a large number of mappings. > diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c > index 4e578d4255f6b..7b883f59221f5 100644 > --- a/drivers/iommu/virtio-iommu.c > +++ b/drivers/iommu/virtio-iommu.c [ ... ] > @@ -656,23 +698,57 @@ static int viommu_domain_map_identity(struct > viommu_endpoint *vdev, > } > > /* > - * viommu_replay_mappings - re-send MAP requests > + * viommu_replay_mappings - send every mapping of the domain again [ ... ] > + while (!done) { > + struct viommu_mapping *mapping; > + struct virtio_iommu_req_map map; [ ... ] > + ret = viommu_add_req(vdomain->viommu, &map, sizeof(map), > &status); > > - ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map)); > - if (ret) { [ ... ] > + if (mapping->iova.last == ULONG_MAX) > + done = true; > + else > + next_iova = mapping->iova.last + 1; > + spin_unlock_irqrestore(&vdomain->mappings_lock, flags); > > - node = interval_tree_iter_next(node, 0, -1UL); > + if (ret) > + break; > } > - spin_unlock_irqrestore(&vdomain->mappings_lock, flags); > > - return ret; > + /* Wait for all of them outside the lock: this is the batched drain */ > + viommu_sync_req(vdomain->viommu); [Severity: Medium] If the virtio-iommu device is removed (e.g., via hot-unplug or driver unbind) while we are queuing these batched requests, could this leak the dynamically allocated struct viommu_request objects? Looking at viommu_sync_req(), it short-circuits when the device is removed without draining the list: drivers/iommu/virtio-iommu.c:viommu_sync_req() { ... if (!viommu_device_live(viommu)) { spin_unlock_irqrestore(&viommu->request_lock, flags); return 0; } ... } Any queued requests would remain in viommu->requests. Since viommu_remove() tears down the virtqueues and sets viommu->removed = true, but does not iterate over the viommu->requests list to free the pending elements, it appears these requests would permanently leak. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
