This resolves the race around touching dev->iommu while generating the OF fwspec on the of_iommu_configure() flow: CPU0 CPU1 of_iommu_configure() iommu_device_register() .. bus_iommu_probe() iommu_fwspec_of_xlate() __iommu_probe_device() iommu_init_device() dev_iommu_get() .. ops->probe fails, no fwspec .. dev_iommu_free() dev->iommu->fwspec *crash* CPU1 is holding the iommu_probe_device_lock for iommu_init_device(), holding it around the of_xlate() and its related manipulation of dev->iommu will close it. The approach also closes a similar race for what should be a successful probe where the above basic construction results in ops->probe observing a partially initialized fwspec. Reported-by: Zhenhua Huang <quic_zhenhuah@xxxxxxxxxxx> Closes: https://lore.kernel.org/linux-arm-kernel/20231017163337.GE282036@xxxxxxxx/T/#mee0d7bdc375541934a571ae69f43b9660f8e7312 Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx> --- drivers/iommu/iommu.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 9cfba9d12d1400..62c82a28cd5db3 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -41,6 +41,7 @@ static struct kset *iommu_group_kset; static DEFINE_IDA(iommu_group_ida); static DEFINE_IDA(iommu_global_pasid_ida); +static DEFINE_MUTEX(iommu_probe_device_lock); static unsigned int iommu_def_domain_type __read_mostly; static bool iommu_dma_strict __read_mostly = IS_ENABLED(CONFIG_IOMMU_DEFAULT_DMA_STRICT); @@ -498,7 +499,6 @@ static int __iommu_probe_device(struct device *dev, struct iommu_fwspec *fwspec = caller_fwspec; const struct iommu_ops *ops; struct iommu_group *group; - static DEFINE_MUTEX(iommu_probe_device_lock); struct group_device *gdev; int ret; @@ -3002,8 +3002,11 @@ int iommu_fwspec_of_xlate(struct iommu_fwspec *fwspec, struct device *dev, if (!fwspec->ops->of_xlate) return -ENODEV; - if (!dev_iommu_get(dev)) + mutex_lock(&iommu_probe_device_lock); + if (!dev_iommu_get(dev)) { + mutex_unlock(&iommu_probe_device_lock); return -ENOMEM; + } /* * ops->of_xlate() requires the fwspec to be passed through dev->iommu, @@ -3015,6 +3018,7 @@ int iommu_fwspec_of_xlate(struct iommu_fwspec *fwspec, struct device *dev, ret = fwspec->ops->of_xlate(dev, iommu_spec); if (dev->iommu->fwspec == fwspec) dev->iommu->fwspec = NULL; + mutex_unlock(&iommu_probe_device_lock); return ret; } @@ -3044,6 +3048,8 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode, struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); int ret; + lockdep_assert_held(&iommu_probe_device_lock); + if (fwspec) return ops == fwspec->ops ? 0 : -EINVAL; @@ -3097,6 +3103,8 @@ int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids) { struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); + lockdep_assert_held(&iommu_probe_device_lock); + if (!fwspec) return -EINVAL; return iommu_fwspec_append_ids(fwspec, ids, num_ids); -- 2.42.0