[Several people have tested this now, so it is something that should sit in
linux-next for a while]
The iommu subsystem uses dev->iommu to store bits of information about the
attached iommu driver. This has been co-opted by the ACPI/OF code to also
be a place to pass around the iommu_fwspec before a driver is probed.
Since both are using the same pointers without any locking it triggers
races if there is concurrent driver loading:
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*
My first attempt get correct locking here was to use the device_lock to
protect the entire *_iommu_configure() and iommu_probe() paths. This
allowed safe use of dev->iommu within those paths. Unfortuately enough
drivers abuse the of_iommu_configure() flow without proper locking and
this approach failed.
This approach removes touches of dev->iommu from the *_iommu_configure()
code. The few remaining required touches are moved into iommu.c and
protected with the existing iommu_probe_device_lock.
To do this we change *_iommu_configure() to hold the iommu_fwspec on the
stack while it is being built. Once it is fully formed the core code will
install it into the dev->iommu when it calls probe.
This also removes all the touches of iommu_ops from
the *_iommu_configure() paths and makes that mechanism private to the
iommu core.
A few more lockdep assertions are added to discourage future mis-use.
This is on github: https://github.com/jgunthorpe/linux/commits/iommu_fwspec
v2:
- Fix all the kconfig randomization 0-day stuff
- Add missing kdoc parameters
- Remove NO_IOMMU, replace it with ENODEV
- Use PTR_ERR to print errno in the new/moved logging
v1: https://lore.kernel.org/r/0-v1-5f734af130a3+34f-iommu_fwspec_jgg@xxxxxxxxxx
Jason Gunthorpe (17):
iommu: Remove struct iommu_ops *iommu from arch_setup_dma_ops()
iommmu/of: Do not return struct iommu_ops from of_iommu_configure()
iommu/of: Use -ENODEV consistently in of_iommu_configure()
acpi: Do not return struct iommu_ops from acpi_iommu_configure_id()
iommu: Make iommu_fwspec->ids a distinct allocation
iommu: Add iommu_fwspec_alloc/dealloc()
iommu: Add iommu_probe_device_fwspec()
iommu/of: Do not use dev->iommu within of_iommu_configure()
iommu: Add iommu_fwspec_append_ids()
acpi: Do not use dev->iommu within acpi_iommu_configure()
iommu: Hold iommu_probe_device_lock while calling ops->of_xlate
iommu: Make iommu_ops_from_fwnode() static
iommu: Remove dev_iommu_fwspec_set()
iommu: Remove pointless iommu_fwspec_free()
iommu: Add ops->of_xlate_fwspec()
iommu: Mark dev_iommu_get() with lockdep
iommu: Mark dev_iommu_priv_set() with a lockdep
arch/arc/mm/dma.c | 2 +-
arch/arm/mm/dma-mapping-nommu.c | 2 +-
arch/arm/mm/dma-mapping.c | 10 +-
arch/arm64/mm/dma-mapping.c | 4 +-
arch/mips/mm/dma-noncoherent.c | 2 +-
arch/riscv/mm/dma-noncoherent.c | 2 +-
drivers/acpi/arm64/iort.c | 42 ++--
drivers/acpi/scan.c | 104 +++++----
drivers/acpi/viot.c | 45 ++--
drivers/hv/hv_common.c | 2 +-
drivers/iommu/amd/iommu.c | 2 -
drivers/iommu/apple-dart.c | 1 -
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +-
drivers/iommu/arm/arm-smmu/arm-smmu.c | 23 +-
drivers/iommu/intel/iommu.c | 2 -
drivers/iommu/iommu.c | 227 +++++++++++++++-----
drivers/iommu/of_iommu.c | 133 +++++-------
drivers/iommu/omap-iommu.c | 1 -
drivers/iommu/tegra-smmu.c | 1 -
drivers/iommu/virtio-iommu.c | 8 +-
drivers/of/device.c | 24 ++-
include/acpi/acpi_bus.h | 8 +-
include/linux/acpi_iort.h | 8 +-
include/linux/acpi_viot.h | 5 +-
include/linux/dma-map-ops.h | 4 +-
include/linux/iommu.h | 47 ++--
include/linux/of_iommu.h | 13 +-
27 files changed, 424 insertions(+), 307 deletions(-)
base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
--
2.42.0
