Re: [PATCH v2 3/6] iommu/virtio: allocate the UNMAP request with the mapping

[sashiko-bot@kernel.org]

> 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 <agicy@qq.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_D757B544955DDC509B5DC7161F425620BA08@qq.com?part=3