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

[Nicolin Chen]

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?

To close that window, I think we might need an ABI contract for the
second case by asking drivers to sync irq and flush the iopf queue?
This would require to roll out driver-level changes as well.

Thanks
Nicolin