Re: [PATCH 2/3] iommufd: WARN if an object is aborted with an elevated refcount

[Nicolin Chen <nicolinc@nvidia.com>]

On Thu, Sep 18, 2025 at 11:47:37AM -0300, Jason Gunthorpe wrote:
> On Wed, Sep 17, 2025 at 11:10:15PM -0700, Nicolin Chen wrote:
> > On Wed, Sep 17, 2025 at 05:01:48PM -0300, Jason Gunthorpe wrote:
> > > If something holds a refcount then it is at risk of UAFing. For abort
> > > paths we expect the caller to never share the object with a parallel
> > > thread and to clean up any refcounts it obtained on its own.
> > > 
> > > Add the missing dec inside iommufd_hwpt_paging_alloc()during error unwind
> > 
> > Space between "()" and "during"
> > 
> > And I don't see this patch touch iommufd_hwpt_paging_alloc(). Is
> > that the iommufd_object_abort() part with the WARN_ON?
> 
> iommufd_hwpt_paging_alloc() calls iommufd_hw_pagetable_detach() so
> this change gives it a put that it didn't have.

Ah, I see.

> > > diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
> > > index 0da2a81eedfa8b..627f9b78483a0e 100644
> > > --- a/drivers/iommu/iommufd/iommufd_private.h
> > > +++ b/drivers/iommu/iommufd/iommufd_private.h
> > > @@ -454,9 +454,8 @@ static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx,
> > >  	if (hwpt->obj.type == IOMMUFD_OBJ_HWPT_PAGING) {
> > >  		struct iommufd_hwpt_paging *hwpt_paging = to_hwpt_paging(hwpt);
> > >  
> > > -		lockdep_assert_not_held(&hwpt_paging->ioas->mutex);
> > > -
> > >  		if (hwpt_paging->auto_domain) {
> > > +			lockdep_assert_not_held(&hwpt_paging->ioas->mutex);
> > >  			iommufd_object_put_and_try_destroy(ictx, &hwpt->obj);
> > >  			return;
> > >  		}
> > 
> > Hmm, this patch doesn't change the scope of ioas-mutex?
> 
> iommufd_hwpt_paging_alloc() now calls this and it knows it doesn't
> pass an auto_domain but it is already under the ioas->mutex in its
> callchain.

I see. This part exists for iommufd_device_change_pt() calling
iommufd_device_do_replace() where auto_domain is the only case
that is ensured to not have ioas->mutex held.

Thanks
Nicolin