Re: [PATCH v5 04/15] iommu/arm-smmu-v3: Drain in-flight fault events on domain detach

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, Sep 28, 2026 at 03:29:02PM -0700, Nicolin Chen wrote:
> On Wed, Sep 23, 2026 at 08:39:20PM -0300, Jason Gunthorpe wrote:
> > On Wed, Sep 23, 2026 at 03:33:06PM -0700, Nicolin Chen wrote:
> > > > But I wonder if the point of this has been lost? Prior to calling the
> > > > driver attach functions the core code already changes the xarray:
> > > > 
> > > > 	curr = xa_cmpxchg(&group->pasid_array, pasid, NULL,
> > > > 			  XA_ZERO_ENTRY, GFP_KERNEL);
> > > > 
> > > > That immediately makes the threaded IRQ safe since it calls
> > > > iommu_attach_handle_get() which now fails.
> > 
> > Hmm, actually that's a sneaky cmpxchg that is only doing reserve..
> > 
> > > I am not sure about that. Looking at iommufd_hwpt_replace_device(),
> > > there can be a old_handle != NULL, in which case the cmpxchg() would
> > > not change the xarray?
> > 
> > I think this is wrong, there is no way it can work like this where the
> > attach continues to see the to-be-detached domain across the
> > flushes. No amount of flushing can fix it.
> > 
> > Somehow we broke it :\
> 
> I'm trying to fix this by replacing xa_cmpxchg() with xa_store().
> 
> However, an asynchronous iommu_attach_handle_get() could return the
> old_handle in the iopf path, before xa_store() in the detach path:
> 
>  - If a different domain is replaced, a driver callback is invoked
>    calling synchronize_irq() and iopf_queue_flush_dev(). And this
>    closes the window on any the old_handle reference before being
>    released.
> 
>  - If a different handle is replaced on the same domain, the driver
>    callback is skipped. Then, the iopf path could hit UAF while the
>    old_handle being released by the detach path?

False alarm. It's actually not reachable today. There is only one
caller from iommufd, where a domain reattachment doesn't swap the
attach handle but bypass that:

	if (hwpt == old_hwpt) {
		mutex_unlock(&igroup->lock);
		return NULL;
	}

I will send a small iommu-core series for review.

Nicolin