[PATCH] iommu: Don't let a driver bind while bus_iommu_probe() still probes its device

[PATCH] iommu: Don't let a driver bind while bus_iommu_probe() still probes its device

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

When an IOMMU driver is a module, udev can load it while the driver of
one of its client devices is loading too. On an Orange Pi 5 (RK3588)
with CONFIG_VSI_IOMMU=m and CONFIG_VIDEO_HANTRO=m, 1 of 30 boots gives:

  hantro-vpu fdc70000.video-codec: late IOMMU probe at driver bind, something fishy here!
  WARNING: drivers/iommu/iommu.c:654 at __iommu_probe_device+0x234/0x5e0
  Call trace:
   __iommu_probe_device+0x234/0x5e0 (P)
   probe_iommu_group+0x40/0x80
   bus_for_each_dev+0x88/0x110
   iommu_device_register+0x10c/0x2d8
   vsi_iommu_probe+0x244/0x280 [vsi_iommu]

"iommus" is an optional fw_devlink supplier, so nothing orders the
client's bind after the IOMMU's probe; that is left to the readiness
check in iommu_fwspec_init(). That check is skipped when dev->iommu is
already set, which is meant to identify the bus_iommu_probe() replay of
dma_configure. But iommu_init_device() allocates dev->iommu and then
drops iommu_probe_device_lock for the replay, so a driver binding the
same device in another context sees it too:

  iommu_device_register()            really_probe(client)
    bus_iommu_probe()
      iommu_init_device()
        dev_iommu_get()
        dev->driver == NULL: replay,
        unlock                         device_set_driver()
        dma_configure()                dma_configure()
                                         of_iommu_configure()
                                           dev->iommu set, or fwspec
                                           set by the replay: return 0
        lock                             iommu_device_use_default_domain()
      dev->driver set: WARN                no group yet: return 0
      group, default domain,           call_driver_probe()
      DMA ops set up under the
      running driver

So the warning is right that something is fishy: the client driver
probes before its device is in a group, with no default domain and with
dma-direct ops that bus_iommu_probe() replaces later. If the client gets
dev->driver set before the replay check instead, bus_iommu_probe() skips
the device, the client defers on the readiness check and its retry does
the probe from the bind path, which warns as well.

Close the window in the pre-probe hook that every bus dma_configure
already calls: in iommu_device_use_default_domain(), a device that has a
fwspec but no group is still being probed (a failed probe frees the
fwspec), so defer the driver as is already done for a group without a
default domain. The driver core retries it once the IOMMU driver has
bound. With that, a driver that is binding but not bound can no longer
run against a half-probed device, so let bus_iommu_probe() replay
dma_configure for it and warn there only for a device that is already
bound. The bind path (iommu_probe_device() from of_iommu_configure())
keeps warning whenever dev->driver is set.

Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
The warning was seen once in 30 boots of an Orange Pi 5 (RK3588) with
the Verisilicon IOMMU and the Hantro decoder built as modules, with
udev loading both at the same time. The trace in the commit message is
from that boot.

Testing: this change on a v7.3 based kernel (drivers/iommu/iommu.c
there is the same as in iommu/next apart from an unrelated
iommu_dma_prepare_msi() hunk) booted 60 times on the same board with
the same modules: no "late IOMMU probe" warning and no other
WARNING/BUG, and in every boot the AV1 and VPU121 decoders were bound
to hantro-vpu and in an IOMMU group, the AV1 decoder's IOMMU was bound
to vsi_iommu, nothing was left in devices_deferred, and the log had no
IOMMU or decoder errors. As the race hit only 1 boot in 30 before, 60
clean boots are weak evidence that it is closed; they mainly show that
IOMMU and client binding still work. The logs do not show whether the
new -EPROBE_DEFER path ran in any boot. No decoding was run in those
boots.

This patch was developed with Claude (Anthropic) as a coding assistant:
it analysed the race and wrote the code and the commit message, and I
reviewed it and tested it as described above.
---
 drivers/iommu/iommu.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b486b8bbd1fc..ec68fb7dbe04 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -481,8 +481,12 @@ static int iommu_init_device(struct device *dev)
 	 * still a big job, so for now just invoke the whole thing. The device
 	 * already having a driver bound means dma_configure has already run and
 	 * found no IOMMU to wait for, so there's no point calling it again.
+	 * A driver that is still binding in another context (dev->driver set,
+	 * but not bound yet) has not got that far: its own dma_configure will
+	 * defer in iommu_device_use_default_domain() until we are done here.
 	 */
-	if (!dev->iommu->fwspec && !dev->driver && dev->bus->dma_configure) {
+	if (!dev->iommu->fwspec && !device_is_bound(dev) &&
+	    dev->bus->dma_configure) {
 		mutex_unlock(&iommu_probe_device_lock);
 		dev->bus->dma_configure(dev);
 		mutex_lock(&iommu_probe_device_lock);
@@ -648,9 +652,13 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
 		return ret;
 	/*
 	 * And if we do now see any replay calls, they would indicate someone
-	 * misusing the dma_configure path outside bus code.
+	 * misusing the dma_configure path outside bus code. For bus_iommu_probe()
+	 * (@group_list set) dev->driver may also be set by a driver which is
+	 * binding concurrently in another context; that driver will defer in
+	 * iommu_device_use_default_domain(), so only a driver that is already
+	 * bound is a late probe there.
 	 */
-	if (dev->driver)
+	if (group_list ? device_is_bound(dev) : !!dev->driver)
 		dev_WARN(dev, "late IOMMU probe at driver bind, something fishy here!\n");
 
 	group = dev->iommu_group;
@@ -3316,9 +3324,26 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
 int iommu_device_use_default_domain(struct device *dev)
 {
 	/* Caller is the driver core during the pre-probe path */
-	struct iommu_group *group = dev->iommu_group;
+	struct iommu_group *group;
+	bool probing;
 	int ret = 0;
 
+	/*
+	 * bus_iommu_probe() in another context may be probing the device right
+	 * now, with iommu_probe_device_lock dropped while it replays
+	 * dma_configure. Our own dma_configure can then see its dev->iommu or
+	 * fwspec and return early, although the device is not in a group yet
+	 * and has no default domain or DMA ops. A fwspec without a group means
+	 * exactly that (a failed probe frees the fwspec), so make the driver
+	 * wait until the probe has finished.
+	 */
+	mutex_lock(&iommu_probe_device_lock);
+	group = dev->iommu_group;
+	probing = !group && dev->iommu && dev->iommu->fwspec;
+	mutex_unlock(&iommu_probe_device_lock);
+
+	if (probing)
+		return -EPROBE_DEFER;
 	if (!group)
 		return 0;
 

---
base-commit: 2cf4be57b301ed2d7af82e70cff5dfd96894d8de
change-id: 20261112-iommu-probe-race-d0c08e77707b

Best regards,
--  
Arun Iyengar <aiyengar1990@gmail.com>

Re: [PATCH] iommu: Don't let a driver bind while bus_iommu_probe() still probes its device

[Arun Iyengar]

Please don't apply v1, and it should not go to stable. On re-reading
I found three problems:

1. "A fwspec without a group means a probe is in progress" does not
   hold on ACPI. On the driver bind path acpi_iommu_configure_id()
   creates a fwspec, and nothing there probes the device or frees the
   fwspec. So a device whose IOMMU probe failed earlier would get
   -EPROBE_DEFER from iommu_device_use_default_domain() on every bind
   attempt, where today it binds without the IOMMU. OF does not have
   this, because of_iommu_configure() either probes or frees.

2. Drivers with driver_managed_dma never call
   iommu_device_use_default_domain(), so the window stays open for
   them (e.g. pcieport with a modular IOMMU driver), and the changed
   WARN condition can then hide it.

3. Replaying dma_configure for a device whose driver is inside probe()
   is wider than I described. It applies to every such device on the
   bus that has no group yet, including a client that went ahead
   without its IOMMU after the deferred probe timeout; that client
   would get its group and default domain while its driver is probing,
   without a warning.

The race itself is as in the commit message: the client's dma_configure
sees the dev->iommu or fwspec of a concurrent bus_iommu_probe() replay
while iommu_probe_device_lock is dropped.

For v2 I intend to mark the in-progress probe explicitly in dev->iommu,
have of_iommu_configure() and acpi_iommu_configure_id() defer any caller
other than the replay while the mark is set, and leave the replay
condition and iommu_device_use_default_domain() alone. The other
ordering in the commit message (the client sets dev->driver before the
replay check) would then still warn, although nothing is wrong there
besides the warning; I would look at that separately. If you would
rather see this solved another way, please say so.

Tudor: you are on Cc because this is the same "!dev->iommu" gate on the
readiness check as in your thread
<20260323-iommu-ready-check-v1-1-5f6fef8f9f59@linaro.org>, for a
different case (one IOMMU, concurrent bind).

Re: [PATCH] iommu: Don't let a driver bind while bus_iommu_probe() still probes its device

[Robin Murphy]

On 2026-11-12 5:32 pm, Arun Iyengar wrote:
> Please don't apply v1, and it should not go to stable. On re-reading
> I found three problems:
> 
> 1. "A fwspec without a group means a probe is in progress" does not
>     hold on ACPI. On the driver bind path acpi_iommu_configure_id()
>     creates a fwspec, and nothing there probes the device or frees the
>     fwspec. So a device whose IOMMU probe failed earlier would get
>     -EPROBE_DEFER from iommu_device_use_default_domain() on every bind
>     attempt, where today it binds without the IOMMU. OF does not have
>     this, because of_iommu_configure() either probes or frees.
> 
> 2. Drivers with driver_managed_dma never call
>     iommu_device_use_default_domain(), so the window stays open for
>     them (e.g. pcieport with a modular IOMMU driver), and the changed
>     WARN condition can then hide it.
> 
> 3. Replaying dma_configure for a device whose driver is inside probe()
>     is wider than I described. It applies to every such device on the
>     bus that has no group yet, including a client that went ahead
>     without its IOMMU after the deferred probe timeout; that client
>     would get its group and default domain while its driver is probing,
>     without a warning.
> 
> The race itself is as in the commit message: the client's dma_configure
> sees the dev->iommu or fwspec of a concurrent bus_iommu_probe() replay
> while iommu_probe_device_lock is dropped.
> 
> For v2 I intend to mark the in-progress probe explicitly in dev->iommu,
> have of_iommu_configure() and acpi_iommu_configure_id() defer any caller
> other than the replay while the mark is set, and leave the replay
> condition and iommu_device_use_default_domain() alone. The other
> ordering in the commit message (the client sets dev->driver before the
> replay check) would then still warn, although nothing is wrong there
> besides the warning; I would look at that separately. If you would
> rather see this solved another way, please say so.
> 
> Tudor: you are on Cc because this is the same "!dev->iommu" gate on the
> readiness check as in your thread
> <20260323-iommu-ready-check-v1-1-5f6fef8f9f59@linaro.org>, for a
> different case (one IOMMU, concurrent bind).

Really the dev->driver checks are less than ideal, as they're inherently 
racy with an unavoidable false-positive window. What we could really do 
with - which might now be viable with the recent atomic device flags - 
is a specific indicator that a device has got far enough into binding a 
driver to be past the point of waiting or an IOMMU. The 
BUS_NOTIFY_BIND_DRIVER call is in fact in more or less the right place 
for that, so we could potentially do the flag management from 
iommu_bus_notifier() rather than clutter up the driver core with yet 
more hooks, but the flag itself can't be in dev->iommu, since its whole 
meaning is "we assumed this device didn't have an IOMMU and now it's too 
late to safely change that".

Thanks,
Robin.