[PATCH v2 11/17] iommu: Hold iommu_probe_device_lock while calling ops->of_xlate

Jason Gunthorpe
2023-11-15T14:06:26Z

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.

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Reported-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
Closes: https://lore.kernel.org/linux-arm-kernel/20231017163337.GE282036@ziepe.ca/T/#mee0d7bdc375541934a571ae69f43b9660f8e7312
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
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 f7bda1c0959d34..5af98cad06f9ef 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;

@@ -2985,8 +2985,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,
@@ -2998,6 +3001,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;
}

@@ -3027,6 +3031,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;

@@ -3080,6 +3086,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