On 2024/2/7 16:11, Tian, Kevin wrote:
From: Lu Baolu <baolu...@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

Reply via email to