> The iommu core states the contract plainly (iommufd pages.c): a driver
> may not fail unmap for reasons beyond bad arguments, and in particular
> may not allocate on the unmap path, because the caller may free the
> memory as soon as unmap returns. viommu_del_mappings() still allocated
> the UNMAP request there, so a forced allocation failure left the device
> mapping a page the driver had already released, while the caller was told
> nothing.
>
> Allocate that request object with the mapping instead - on the map path,
> which is allowed to fail and reports the failure to the caller that is
> mapping - and free it with the mapping when no UNMAP is sent: the unmap
> path then only fills in and queues a request. Measured with a fault
> injected into that allocation (failslab, on the fault-injection harness
> this work used): the unpatched driver returns success for a re-map of the
> same IOVA while the device still translates it to the freed page, proven
> with a real DMA through the QEMU iommu-testdev; with this patch the same
> scenario reports no such case in any round and the DMA lands on the new
> page. The unmap path allocates nothing afterwards, and the device sees
> the same request stream.
>
> Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
> Signed-off-by: Anlai Lu <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review ยท
https://sashiko.dev/#/patchset/[email protected]?part=3