Re: [PATCH 00/14] iommu: fix device leaks

[Jason Gunthorpe <jgg@ziepe.ca>]

On Thu, Oct 02, 2025 at 12:48:35PM +0100, Robin Murphy wrote:
> > However the SMMU drivers are doing this under the
> > iommu_probe_device_lock and not stashing the pointer into a drvdata
> > where there is no locking protecting it.
> 
> Huh? Every .of_xlate call is under probe_device_lock just as much as
> .probe_device calls are; they *have* to be, since they too are in the
> position of working with dev->iommu before dev->iommu_group is set to
> guarantee its stability.

Yes, of_xlate is under the lock, but IIRC there are still cases where
probe gets deferred, the probe_device_lock is unlocked, and the
drvdata continues to exist.

> indeed the driver cannot be removed, because we hold a module reference
> around the call

That's not how module reference counts work. Drivers can be unbound
through sysfs at any time.

> > Anyhow, I drafted a nice fix for all of this. After all the rework it
> > is trivial for the core code to pass in the struct iommu_device * to
> > probe and then most of the drivers just drop this ugly code
> > completely.
> >
> > https://github.com/jgunthorpe/linux/commits/iommu-fwspec/
> 
> Eww, that is neither nice nor a "fix". Once again it's just piling on a load
> of extra complexity to have multiple confusingly-overlapping-but-different
> ways of doing the same thing, one of which is still the exact same one
> you've decided to object to because you've failed to understand it (as
> demonstrated by the commit message below, the obvious bug in the hideous
> mess below that, and at a glance the other patches actively *breaking* at
> least one driver.)

It is hard to take you seriously with such vauge objections
Robin. Please try to be constructive. If you are specific I'll go fix
the things and maybe other people besides you can understand this
stuff.

I'm shocked you disagree with the core code helping the drivers find
their iommu_driver. This seems like very basic obvious good stuff. We
don't need all sorts of different open coded ugly ways for drivers to
do this. It removes 200 lines of junk for drivers :\

> > +			iommu = fwspec_iommu->ops->probe_device(dev);
> > +			if (IS_ERR(iommu))
> > +				return PTR_ERR(iommu);
> > +			if (WARN_ON(iommu != fwspec_iommu)) {

This is at least one typo.

Jason