On Sun, Feb 26, 2023 at 11:01:59AM +0800, Baolu Lu wrote: > On 2/25/23 8:27 AM, Jason Gunthorpe wrote: > > @@ -437,25 +517,77 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id) > > struct iommufd_ioas *ioas = > > container_of(pt_obj, struct iommufd_ioas, obj); > > - rc = iommufd_device_auto_get_domain(idev, ioas, pt_id); > > - if (rc) > > + destroy_hwpt = iommufd_device_auto_get_domain(idev, ioas, pt_id, > > + do_attach); > > + if (IS_ERR(destroy_hwpt)) > > goto out_put_pt_obj; > > break; > > } > > default: > > - rc = -EINVAL; > > + destroy_hwpt = ERR_PTR(-EINVAL); > > goto out_put_pt_obj; > > } > > + iommufd_put_object(pt_obj); > > - refcount_inc(&idev->obj.users); > > - rc = 0; > > + /* This destruction has to be after we unlock everything */ > > + if (destroy_hwpt) > > Should this be > > if (!IS_ERR_OR_NULL(destroy_hwpt)) > > ? Never use IS_ERR_OR_NULL .. What am I missing? all the flows that could possibly have err_ptr here do goto_out_put_pt_obj ? Jason
