Messages in this thread | | | Date | Wed, 21 Feb 2024 13:52:39 +0800 | Subject | Re: [PATCH v3 1/8] iommu: Add iopf domain attach/detach/replace interface | From | Baolu Lu <> |
| |
On 2024/2/7 16:11, Tian, Kevin wrote: >> From: Lu Baolu <baolu.lu@linux.intel.com> >> Sent: Monday, January 22, 2024 3:39 PM >> >> There is a slight difference between iopf domains and non-iopf domains. >> In the latter, references to domains occur between attach and detach; >> While in the former, due to the existence of asynchronous iopf handling >> paths, references to the domain may occur after detach, which leads to >> potential UAF issues. > > Does UAF still exist if iommu driver follows the guidance you just added > to iopf_queue_remove_device()? > > it clearly says that the driver needs to disable IOMMU PRI reception, > remove device from iopf queue and disable PRI on the device.
The iopf_queue_remove_device() function is only called after the last iopf-capable domain is detached from the device. It may not be called during domain replacement. Hence, there is no guarantee that iopf_queue_remove_device() will be called when a domain is detached from the device.
> > presumably those are all about what needs to be done in the detach > operation. Then once detach completes there should be no more > reference to the domain from the iopf path?
The domain pointer stored in the iopf_group structure is only released after the iopf response, possibly after the domain is detached from the device. Thus, the domain pointer can only be freed after the iopf response.
> >> >> +struct iopf_attach_cookie { >> + struct iommu_domain *domain; >> + struct device *dev; >> + unsigned int pasid; >> + refcount_t users; >> + >> + void *private; >> + void (*release)(struct iopf_attach_cookie *cookie); >> +}; > > this cookie has nothing specific to iopf. > > it may makes more sense to build a generic iommu_attach_device_cookie() > helper so the same object can be reused in future other usages too. > > within iommu core it can check domain iopf handler and this generic cookie > to update iopf specific data e.g. the pasid_cookie xarray.
This means attaching an iopf-capable domain follows two steps:
1) Attaching the domain to the device. 2) Setting up the iopf data, necessary for handling iopf data.
This creates a time window during which the iopf is enabled, but the software cannot handle it. Or not?
Best regards, baolu
| |