On 2024/2/20 21:57, Joel Granados wrote:
diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
index e752d1c49dde..a4a49f3cd4c2 100644
--- a/drivers/iommu/iommufd/fault.c
+++ b/drivers/iommu/iommufd/fault.c
@@ -267,3 +267,125 @@ int iommufd_fault_iopf_handler(struct iopf_group *group)
return 0;
}
+
+static void release_attach_cookie(struct iopf_attach_cookie *cookie)
+{
+ struct iommufd_hw_pagetable *hwpt = cookie->domain->fault_data;
There is a possibility here of cookie->domain being NULL. When you call
release_attach_cookie from iommufd_fault_domain_attach_dev if
idev->iopf_enabled is false. In this case, you have not set the domain
yet.
Yes. Good catch!
+ struct iommufd_device *idev = cookie->private;
+
+ refcount_dec(&idev->obj.users);
+ refcount_dec(&hwpt->obj.users);
You should decrease this ref count only if the cookie actually had a
domain.
This function could be something like this:
static void release_attach_cookie(struct iopf_attach_cookie *cookie)
{
struct iommufd_hw_pagetable *hwpt;
struct iommufd_device *idev = cookie->private;
refcount_dec(&idev->obj.users);
if (cookie->domain) {
hwpt = cookie->domain->fault_data;
refcount_dec(&hwpt->obj.users);
}
kfree(cookie);
}
Yeah, fixed.
+ kfree(cookie);
+}
+
+static int iommufd_fault_iopf_enable(struct iommufd_device *idev)
+{
+ int ret;
+
+ if (idev->iopf_enabled)
+ return 0;
+
+ ret = iommu_dev_enable_feature(idev->dev, IOMMU_DEV_FEAT_IOPF);
+ if (ret)
+ return ret;
+
+ idev->iopf_enabled = true;
+
+ return 0;
+}
+
+static void iommufd_fault_iopf_disable(struct iommufd_device *idev)
+{
+ if (!idev->iopf_enabled)
+ return;
+
+ iommu_dev_disable_feature(idev->dev, IOMMU_DEV_FEAT_IOPF);
+ idev->iopf_enabled = false;
+}
+
+int iommufd_fault_domain_attach_dev(struct iommufd_hw_pagetable *hwpt,
+ struct iommufd_device *idev)
+{
+ struct iopf_attach_cookie *cookie;
+ int ret;
+
+ cookie = kzalloc(sizeof(*cookie), GFP_KERNEL);
+ if (!cookie)
+ return -ENOMEM;
+
+ refcount_inc(&hwpt->obj.users);
+ refcount_inc(&idev->obj.users);
+ cookie->release = release_attach_cookie;
+ cookie->private = idev;
+
+ if (!idev->iopf_enabled) {
+ ret = iommufd_fault_iopf_enable(idev);
+ if (ret)
+ goto out_put_cookie;
You have not set domain here and release_attach_cookie will try to
access a null address.
Fixed as above.
Best regards,
baolu
