[PATCH v5 0/7] iommu: Standardize ATS robustness and state tracking

[PATCH v5 0/7] iommu: Standardize ATS robustness and state tracking

[Pranjal Shrivastava]

The primary motivation for this series is an ATS state mismatch observed
under heavy load (via iova_stress). A failure in pci_enable_ats() leaves
IOMMU drivers like arm-smmu-v3 with inconsistent state leading to PCI core
warnings during device detach.

While David's recent work [1] addressed a discovery race for specific
quirked devices by moving them to the HEADER phase, gaps remained
regarding how Virtual Functions (VFs) inherit state from their Physical
Functions (PFs). Specifically, pci_ats_supported() did not account for
PF-level quirked status, and pci_prepare_ats() lacked STU validation for
VFs.

Based on discussion with Jason in v3 it was decided that IOMMU drivers 
should explicitly check pci_ats_supported before calling pci_prepare_ats
To facilitate this, pci_prepare_ats() has been decoupled from the internal
support check. Furthermore, the device probe should fail if pci_prepare_ats()
fails. Since these early gates preclude software configuration errors,
any remaining failure during pci_enable_ats() is treated as a kernel bug

This series standardizes this pattern across ARM SMMUv3, Intel VT-d, &
AMD IOMMU drivers, while also hardening the latter two against pre-existing
issues found by Sashiko in their probe error paths identified during review.

[1] https://lore.kernel.org/linux-pci/20260403222750.1215002-1-dmatlack@google.com/

[v5]
 - Decoupled pci_ats_supported() from pci_prepare_ats() in the PCI core.
 - Rebased SMMUv3 support on top of Nicolin's "Always-On ATS" series.
 - Fixed pre-existing RB-tree corruption in VT-d probe (Baolu/Sashiko).
 - Addressed the pre-existing UAF in AMD IOMMU probe suggested by Sashiko
 - Collected R-b tags from Nicolin.

[v4]
 - https://lore.kernel.org/all/20260525184347.4059549-1-praan@google.com/
 - Standardized the pattern across Intel VT-d and AMD IOMMU drivers.
 - Replaced the SMMUv3 ats_prepared gate with a fatal probe-fail logic.
 - Utilized WARN() macros for runtime enablement failures in all drivers.
 - Collected R-b tags from Jason and Sami.

[v3] https://lore.kernel.org/all/20260519135323.1558777-1-praan@google.com/
[v2] https://lore.kernel.org/all/20260504163842.2692314-1-praan@google.com/


Pranjal Shrivastava (7):
  PCI/ATS: Ensure pci_ats_supported() is PF-aware for VFs
  PCI/ATS: Validate STU for VFs in pci_prepare_ats()
  PCI/ATS: Decouple pci_ats_supported() from pci_prepare_ats()
  iommu/arm-smmu-v3: Standardize ATS enablement failure reporting
  iommu/vt-d: Fix RB-tree corruption and Use-After-Free in probe
  iommu/vt-d: Fail probe on ATS configuration failure
  iommu/amd: Fail probe on ATS configuration failure

 drivers/iommu/amd/iommu.c                   | 30 ++++++++++++++++-----
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 10 +++++--
 drivers/iommu/intel/iommu.c                 | 22 ++++++++++++---
 drivers/pci/ats.c                           | 21 ++++++++++-----
 4 files changed, 64 insertions(+), 19 deletions(-)

-- 
2.54.0.823.g6e5bcc1fc9-goog

[PATCH v5 1/7] PCI/ATS: Ensure pci_ats_supported() is PF-aware for VFs

[Pranjal Shrivastava]

Update pci_ats_supported() to additionally check the associated PF's
status when called on a VF. This ensures that PF-level quirks and
untrusted status are correctly propagated to VFs, providing a robust
support check that aligns with the kernel's PF-centric ATS configuration
model and is immune to the timing of VF-specific fixups.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/pci/ats.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 96efa00d9743..679a3c3c1d54 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -40,10 +40,13 @@ void pci_ats_init(struct pci_dev *dev)
  */
 bool pci_ats_supported(struct pci_dev *dev)
 {
-	if (!dev->ats_cap)
+	if (!dev->ats_cap || dev->untrusted)
 		return false;
 
-	return (dev->untrusted == 0);
+	if (dev->is_virtfn)
+		return pci_ats_supported(pci_physfn(dev));
+
+	return true;
 }
 EXPORT_SYMBOL_GPL(pci_ats_supported);
 
-- 
2.54.0.823.g6e5bcc1fc9-goog

Re: [PATCH v5 1/7] PCI/ATS: Ensure pci_ats_supported() is PF-aware for VFs

[Baolu Lu]

On 5/29/26 04:23, Pranjal Shrivastava wrote:
> Update pci_ats_supported() to additionally check the associated PF's
> status when called on a VF. This ensures that PF-level quirks and
> untrusted status are correctly propagated to VFs, providing a robust
> support check that aligns with the kernel's PF-centric ATS configuration
> model and is immune to the timing of VF-specific fixups.
> 
> Reviewed-by: Jason Gunthorpe<jgg@nvidia.com>
> Reviewed-by: Samiullah Khawaja<skhawaja@google.com>
> Reviewed-by: Nicolin Chen<nicolinc@nvidia.com>
> Signed-off-by: Pranjal Shrivastava<praan@google.com>
> ---
>   drivers/pci/ats.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

[PATCH v5 2/7] PCI/ATS: Validate STU for VFs in pci_prepare_ats()

[Pranjal Shrivastava]

While every PCI Function that implements ATS has an independent ATS
Extended Capability structure with a Read/Write Smallest Translation
Unit (STU) field, the kernel manages SR-IOV ATS by requiring the IOMMU
driver to configure the STU on the Physical Function (PF) before any
any Virtual Functions (VFs) are created.

Currently, pci_prepare_ats() bails out early for VFs, assuming that the
PF has already been correctly prepared. However, this creates a potential
mismatch if a VF is subsequently prepared with a different page shift.

Update pci_prepare_ats() to validate that the requested page shift (ps)
matches the STU already configured in the associated PF. This ensures
early detection of incompatible configurations and maintains the kernel's
policy of consistent STU sizing across all functions associated with a
given SMMU.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/pci/ats.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 679a3c3c1d54..8057c24b0469 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -73,8 +73,13 @@ int pci_prepare_ats(struct pci_dev *dev, int ps)
 	if (ps < PCI_ATS_MIN_STU)
 		return -EINVAL;
 
-	if (dev->is_virtfn)
+	if (dev->is_virtfn) {
+		struct pci_dev *pdev = pci_physfn(dev);
+
+		if (pdev->ats_stu != ps)
+			return -EINVAL;
 		return 0;
+	}
 
 	dev->ats_stu = ps;
 	ctrl = PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU);
-- 
2.54.0.823.g6e5bcc1fc9-goog

Re: [PATCH v5 2/7] PCI/ATS: Validate STU for VFs in pci_prepare_ats()

[Baolu Lu]

On 5/29/26 04:23, Pranjal Shrivastava wrote:
> While every PCI Function that implements ATS has an independent ATS
> Extended Capability structure with a Read/Write Smallest Translation
> Unit (STU) field, the kernel manages SR-IOV ATS by requiring the IOMMU
> driver to configure the STU on the Physical Function (PF) before any
> any Virtual Functions (VFs) are created.
> 
> Currently, pci_prepare_ats() bails out early for VFs, assuming that the
> PF has already been correctly prepared. However, this creates a potential
> mismatch if a VF is subsequently prepared with a different page shift.
> 
> Update pci_prepare_ats() to validate that the requested page shift (ps)
> matches the STU already configured in the associated PF. This ensures
> early detection of incompatible configurations and maintains the kernel's
> policy of consistent STU sizing across all functions associated with a
> given SMMU.
> 
> Reviewed-by: Jason Gunthorpe<jgg@nvidia.com>
> Reviewed-by: Samiullah Khawaja<skhawaja@google.com>
> Reviewed-by: Nicolin Chen<nicolinc@nvidia.com>
> Signed-off-by: Pranjal Shrivastava<praan@google.com>
> ---
>   drivers/pci/ats.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

[PATCH v5 3/7] PCI/ATS: Decouple pci_ats_supported() from pci_prepare_ats()

[Pranjal Shrivastava]

Currently, pci_prepare_ats() internally calls pci_ats_supported() and
returns -EINVAL if the device does not support ATS. While this provides
a safety check, it conflates support detection with configuration.

Update pci_prepare_ats() to remove the internal support check. This
decouples support verification from the configuration phase, ensuring
that drivers can distinguish between a device that does not support ATS
and one that has a true configuration error (e.g. STU mismatch).

Update the function documentation to mandate that callers must verify
ATS support (via pci_ats_supported()) before calling pci_prepare_ats().

Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/pci/ats.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 8057c24b0469..92c2a6bc2dcc 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -56,7 +56,9 @@ EXPORT_SYMBOL_GPL(pci_ats_supported);
  * @ps: the IOMMU page shift
  *
  * This must be done by the IOMMU driver on the PF before any VFs are created to
- * ensure that the VF can have ATS enabled.
+ * ensure that the VF can have ATS enabled. Callers must verify that ATS is
+ * supported by the device (e.g. via pci_ats_supported()) before calling this
+ * function.
  *
  * Returns 0 on success, or negative on failure.
  */
@@ -64,9 +66,6 @@ int pci_prepare_ats(struct pci_dev *dev, int ps)
 {
 	u16 ctrl;
 
-	if (!pci_ats_supported(dev))
-		return -EINVAL;
-
 	if (WARN_ON(dev->ats_enabled))
 		return -EBUSY;
 
-- 
2.54.0.823.g6e5bcc1fc9-goog

Re: [PATCH v5 3/7] PCI/ATS: Decouple pci_ats_supported() from pci_prepare_ats()

[Baolu Lu]

On 5/29/26 04:23, Pranjal Shrivastava wrote:
> Currently, pci_prepare_ats() internally calls pci_ats_supported() and
> returns -EINVAL if the device does not support ATS. While this provides
> a safety check, it conflates support detection with configuration.
> 
> Update pci_prepare_ats() to remove the internal support check. This
> decouples support verification from the configuration phase, ensuring
> that drivers can distinguish between a device that does not support ATS
> and one that has a true configuration error (e.g. STU mismatch).
> 
> Update the function documentation to mandate that callers must verify
> ATS support (via pci_ats_supported()) before calling pci_prepare_ats().
> 
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
>   drivers/pci/ats.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index 8057c24b0469..92c2a6bc2dcc 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -56,7 +56,9 @@ EXPORT_SYMBOL_GPL(pci_ats_supported);
>    * @ps: the IOMMU page shift
>    *
>    * This must be done by the IOMMU driver on the PF before any VFs are created to
> - * ensure that the VF can have ATS enabled.
> + * ensure that the VF can have ATS enabled. Callers must verify that ATS is
> + * supported by the device (e.g. via pci_ats_supported()) before calling this
> + * function.
>    *
>    * Returns 0 on success, or negative on failure.
>    */
> @@ -64,9 +66,6 @@ int pci_prepare_ats(struct pci_dev *dev, int ps)
>   {
>   	u16 ctrl;
>   
> -	if (!pci_ats_supported(dev))
> -		return -EINVAL;
> -
>   	if (WARN_ON(dev->ats_enabled))
>   		return -EBUSY;
>   

I am not sure that the removal above ensures that 'drivers can
distinguish between a device that does not support ATS and one that has
a true configuration error (e.g., STU mismatch)', especially considering
that this helper already has a return value that explicitly conveys the
failure reason.

Furthermore, if a caller misuses this API by calling it against a non-
ATS device, the following code executes:

         ctrl = PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU);
         pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl);

This causes the driver to attempt a write to an invalid or non-existent
PCI configuration space address. Instead of removing the check from the
function entirely, how about adding a WARN_ON() around it?

	if (WARN_ON(!pci_ats_supported(dev)))
		return -EINVAL;

Thanks,
baolu

Re: [PATCH v5 3/7] PCI/ATS: Decouple pci_ats_supported() from pci_prepare_ats()

[Pranjal Shrivastava]

On Fri, May 29, 2026 at 02:29:16PM +0800, Baolu Lu wrote:
> On 5/29/26 04:23, Pranjal Shrivastava wrote:
> > Currently, pci_prepare_ats() internally calls pci_ats_supported() and
> > returns -EINVAL if the device does not support ATS. While this provides
> > a safety check, it conflates support detection with configuration.
> > 
> > Update pci_prepare_ats() to remove the internal support check. This
> > decouples support verification from the configuration phase, ensuring
> > that drivers can distinguish between a device that does not support ATS
> > and one that has a true configuration error (e.g. STU mismatch).
> > 
> > Update the function documentation to mandate that callers must verify
> > ATS support (via pci_ats_supported()) before calling pci_prepare_ats().
> > 
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > ---
> >   drivers/pci/ats.c | 7 +++----
> >   1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> > index 8057c24b0469..92c2a6bc2dcc 100644
> > --- a/drivers/pci/ats.c
> > +++ b/drivers/pci/ats.c
> > @@ -56,7 +56,9 @@ EXPORT_SYMBOL_GPL(pci_ats_supported);
> >    * @ps: the IOMMU page shift
> >    *
> >    * This must be done by the IOMMU driver on the PF before any VFs are created to
> > - * ensure that the VF can have ATS enabled.
> > + * ensure that the VF can have ATS enabled. Callers must verify that ATS is
> > + * supported by the device (e.g. via pci_ats_supported()) before calling this
> > + * function.
> >    *
> >    * Returns 0 on success, or negative on failure.
> >    */
> > @@ -64,9 +66,6 @@ int pci_prepare_ats(struct pci_dev *dev, int ps)
> >   {
> >   	u16 ctrl;
> > -	if (!pci_ats_supported(dev))
> > -		return -EINVAL;
> > -
> >   	if (WARN_ON(dev->ats_enabled))
> >   		return -EBUSY;
> 
> I am not sure that the removal above ensures that 'drivers can
> distinguish between a device that does not support ATS and one that has
> a true configuration error (e.g., STU mismatch)', especially considering
> that this helper already has a return value that explicitly conveys the
> failure reason.
> 
> Furthermore, if a caller misuses this API by calling it against a non-
> ATS device,

Ack. The idea was that all callers will check pci_ats_supported() before
calling pci_prepare_ats() but I guess I didn't consider callers abusing 
this for non-ATS cases.

> 
>         ctrl = PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU);
>         pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl);
> 
> This causes the driver to attempt a write to an invalid or non-existent
> PCI configuration space address. Instead of removing the check from the
> function entirely, how about adding a WARN_ON() around it?
> 
> 	if (WARN_ON(!pci_ats_supported(dev)))
> 		return -EINVAL;

Ack. That makes sense. I'll post another version!

Thanks,
Praan

[PATCH v5 4/7] iommu/arm-smmu-v3: Standardize ATS enablement failure reporting

[Pranjal Shrivastava]

The SMMUv3 driver currently has a two-phase commit in its ATS enablement
flow. During arm_smmu_attach_prepare(), it predicts whether ATS will be
enabled using arm_smmu_ats_supported() and accordingly increments
nr_ats_masters and merges ATS invalidations into the domain's invs array.

However, the actual hardware enablement via pci_enable_ats() happens
later in arm_smmu_attach_commit(). If this call to pci_enable_ats fails,
the SMMU driver's ATS state tracking remains polluted, i.e., the driver
tracks ATS as enabled on a master that is not actually using it. This
leads to an incorrect nr_ats_masters and triggers a warning in the PCI
core during detach:

 1 [  127.925080] ------------[ cut here ]------------
 2 [  127.925084] WARNING: drivers/pci/ats.c:132 at pci_disable_ats+0x94/0xa8
 3 ...
 4 [  128.068169] Call trace:
 5 [  128.070603]  pci_disable_ats+0x94/0xa8 (P)
 6 [  128.074688]  arm_smmu_attach_prepare+0x104/0x310
 7 [  128.079292]  arm_smmu_attach_dev_ste+0x128/0x1e0

The issue was exposed under heavy load when running a VFIO-based DMA
map stress test (iova_stress).

Following the addition of the arm_smmu_master_prepare_ats() [1] helper during
device probe, failable ATS configuration (STU setup) is now handled early
during probe. This ensures that any master reaching the attach phase is
guaranteed to have a valid ATS configuration.

Update arm_smmu_enable_ats() to use the WARN() macro for any
subsequent enablement failures during the commit phase. Since probe
checks now preclude software configuration errors, any failure here is
considered a kernel bug.

[1] https://lore.kernel.org/all/cover.1779392420.git.nicolinc@nvidia.com/

Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index a10affb483a4..aaebd72bc48d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2956,8 +2956,14 @@ static void arm_smmu_enable_ats(struct arm_smmu_master *master)
 	 * ATC invalidation of PASID 0 causes the entire ATC to be flushed.
 	 */
 	arm_smmu_atc_inv_master(master, IOMMU_NO_PASID);
-	if (pci_enable_ats(pdev, stu))
-		dev_err(master->dev, "Failed to enable ATS (STU %zu)\n", stu);
+
+	/*
+	 * Any failure at this point is a kernel bug. pci_ats_supported()
+	 * and pci_prepare_ats() have already verified the hardware capability
+	 * and programmed the STU. Thus, pci_enable_ats() should not fail here.
+	 */
+	WARN(pci_enable_ats(pdev, stu),
+	     "Failed to enable ATS (STU %zu)\n", stu);
 }
 
 static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
-- 
2.54.0.823.g6e5bcc1fc9-goog

[PATCH v5 5/7] iommu/vt-d: Fix RB-tree corruption and Use-After-Free in probe

[Pranjal Shrivastava]

The intel_iommu_probe_device() function contains two pre-existing
memory safety issues on its error path:

1. The info->node RB-tree member is zero-initialized via kzalloc. If
   a device does not support ATS, the device_rbtree_insert() call is
   skipped. If a subsequent probe step fails, the error path jumps to
   device_rbtree_remove(), which misinterprets the zeroed node as
   a tree root and corrupts the device RB-tree.

2. The info structure is freed on failure, but the pointer remains
   linked to the device via dev_iommu_priv_set(). This leads to a
   Use-After-Free regression if the pointer is accessed later.

Fix these by explicitly initializing the RB-node as empty and guarding
its removal. Additionally, ensure dev_iommu_priv_set(dev, NULL) is
called before freeing the info structure in the error path.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260525205628.CD4431F000E9@smtp.kernel.org/
Suggested-by: Baolu Lu <baolu.lu@linux.intel.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/intel/iommu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 4d0e65bc131d..ed6d3a0203f5 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -157,7 +157,10 @@ static void device_rbtree_remove(struct device_domain_info *info)
 	unsigned long flags;
 
 	spin_lock_irqsave(&iommu->device_rbtree_lock, flags);
-	rb_erase(&info->node, &iommu->device_rbtree);
+	if (!RB_EMPTY_NODE(&info->node)) {
+		rb_erase(&info->node, &iommu->device_rbtree);
+		RB_CLEAR_NODE(&info->node);
+	}
 	spin_unlock_irqrestore(&iommu->device_rbtree_lock, flags);
 }
 
@@ -3254,6 +3257,7 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
 
 	info->dev = dev;
 	info->iommu = iommu;
+	RB_CLEAR_NODE(&info->node);
 	if (dev_is_pci(dev)) {
 		if (ecap_dev_iotlb_support(iommu->ecap) &&
 		    pci_ats_supported(pdev) &&
@@ -3316,6 +3320,7 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
 clear_rbtree:
 	device_rbtree_remove(info);
 free:
+	dev_iommu_priv_set(dev, NULL);
 	kfree(info);
 
 	return ERR_PTR(ret);
-- 
2.54.0.823.g6e5bcc1fc9-goog

Re: [PATCH v5 5/7] iommu/vt-d: Fix RB-tree corruption and Use-After-Free in probe

[Baolu Lu]

On 5/29/26 04:23, Pranjal Shrivastava wrote:
> The intel_iommu_probe_device() function contains two pre-existing
> memory safety issues on its error path:
> 
> 1. The info->node RB-tree member is zero-initialized via kzalloc. If
>     a device does not support ATS, the device_rbtree_insert() call is
>     skipped. If a subsequent probe step fails, the error path jumps to
>     device_rbtree_remove(), which misinterprets the zeroed node as
>     a tree root and corrupts the device RB-tree.
> 
> 2. The info structure is freed on failure, but the pointer remains
>     linked to the device via dev_iommu_priv_set(). This leads to a
>     Use-After-Free regression if the pointer is accessed later.
> 
> Fix these by explicitly initializing the RB-node as empty and guarding
> its removal. Additionally, ensure dev_iommu_priv_set(dev, NULL) is
> called before freeing the info structure in the error path.

Thanks for the fixes. Could you please separate these two fixes into two
distinct patches and post them as a standalone series? These two fixes
are quick cleanups and are not part of the current series, which focuses
on improving the robustness of ATS enablement.

> 
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/all/20260525205628.CD4431F000E9@smtp.kernel.org/
> Suggested-by: Baolu Lu <baolu.lu@linux.intel.com>
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
>   drivers/iommu/intel/iommu.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 4d0e65bc131d..ed6d3a0203f5 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -157,7 +157,10 @@ static void device_rbtree_remove(struct device_domain_info *info)
>   	unsigned long flags;
>   
>   	spin_lock_irqsave(&iommu->device_rbtree_lock, flags);
> -	rb_erase(&info->node, &iommu->device_rbtree);
> +	if (!RB_EMPTY_NODE(&info->node)) {
> +		rb_erase(&info->node, &iommu->device_rbtree);
> +		RB_CLEAR_NODE(&info->node);
> +	}
>   	spin_unlock_irqrestore(&iommu->device_rbtree_lock, flags);
>   }
>   
> @@ -3254,6 +3257,7 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
>   
>   	info->dev = dev;
>   	info->iommu = iommu;
> +	RB_CLEAR_NODE(&info->node);
>   	if (dev_is_pci(dev)) {
>   		if (ecap_dev_iotlb_support(iommu->ecap) &&
>   		    pci_ats_supported(pdev) &&
> @@ -3316,6 +3320,7 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
>   clear_rbtree:
>   	device_rbtree_remove(info);
>   free:
> +	dev_iommu_priv_set(dev, NULL);
>   	kfree(info);
>   
>   	return ERR_PTR(ret);

Thanks,
baolu

Re: [PATCH v5 5/7] iommu/vt-d: Fix RB-tree corruption and Use-After-Free in probe

[Pranjal Shrivastava]

On Fri, May 29, 2026 at 11:20:47AM +0800, Baolu Lu wrote:
> On 5/29/26 04:23, Pranjal Shrivastava wrote:
> > The intel_iommu_probe_device() function contains two pre-existing
> > memory safety issues on its error path:
> > 
> > 1. The info->node RB-tree member is zero-initialized via kzalloc. If
> >     a device does not support ATS, the device_rbtree_insert() call is
> >     skipped. If a subsequent probe step fails, the error path jumps to
> >     device_rbtree_remove(), which misinterprets the zeroed node as
> >     a tree root and corrupts the device RB-tree.
> > 
> > 2. The info structure is freed on failure, but the pointer remains
> >     linked to the device via dev_iommu_priv_set(). This leads to a
> >     Use-After-Free regression if the pointer is accessed later.
> > 
> > Fix these by explicitly initializing the RB-node as empty and guarding
> > its removal. Additionally, ensure dev_iommu_priv_set(dev, NULL) is
> > called before freeing the info structure in the error path.
> 
> Thanks for the fixes. Could you please separate these two fixes into two
> distinct patches and post them as a standalone series? These two fixes
> are quick cleanups and are not part of the current series, which focuses
> on improving the robustness of ATS enablement.

Ack. I'll send these as stanalone patches. I added these here to keep
Sashiko at bay.

Thanks,
Praan

[PATCH v5 6/7] iommu/vt-d: Fail probe on ATS configuration failure

[Pranjal Shrivastava]

Update the Intel VT-d driver to handle ATS configuration and enablement
more strictly. Specifically, update the device probe to fail if
pci_prepare_ats() returns an error. This ensures that any ATS-capable
master reaching the attach phase is guaranteed to have a valid config.

Additionally, update iommu_enable_pci_ats() to WARN() if pci_enable_ats
fails. Since earlier checks in the probe phase preclude config-related
failures, any failure during hardware enablement is considered a kernel
bug.

Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/intel/iommu.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ed6d3a0203f5..f13da16717fe 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -876,8 +876,14 @@ static void iommu_enable_pci_ats(struct device_domain_info *info)
 	if (!pci_ats_page_aligned(pdev))
 		return;
 
-	if (!pci_enable_ats(pdev, VTD_PAGE_SHIFT))
-		info->ats_enabled = 1;
+	/*
+	 * pci_enable_ats() should not fail here because earlier checks
+	 * have already verified support and configuration.
+	 */
+	if (WARN_ON(pci_enable_ats(pdev, VTD_PAGE_SHIFT)))
+		return;
+
+	info->ats_enabled = 1;
 }
 
 static void iommu_disable_pci_ats(struct device_domain_info *info)
@@ -3292,7 +3298,10 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
 
 	dev_iommu_priv_set(dev, info);
 	if (pdev && pci_ats_supported(pdev)) {
-		pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
+		ret = pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
+		if (ret)
+			goto free;
+
 		ret = device_rbtree_insert(iommu, info);
 		if (ret)
 			goto free;
-- 
2.54.0.823.g6e5bcc1fc9-goog

Re: [PATCH v5 6/7] iommu/vt-d: Fail probe on ATS configuration failure

[Baolu Lu]

On 5/29/26 04:23, Pranjal Shrivastava wrote:
> Update the Intel VT-d driver to handle ATS configuration and enablement
> more strictly. Specifically, update the device probe to fail if
> pci_prepare_ats() returns an error. This ensures that any ATS-capable
> master reaching the attach phase is guaranteed to have a valid config.
> 
> Additionally, update iommu_enable_pci_ats() to WARN() if pci_enable_ats
> fails. Since earlier checks in the probe phase preclude config-related
> failures, any failure during hardware enablement is considered a kernel
> bug.
> 
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
>   drivers/iommu/intel/iommu.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index ed6d3a0203f5..f13da16717fe 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -876,8 +876,14 @@ static void iommu_enable_pci_ats(struct device_domain_info *info)
>   	if (!pci_ats_page_aligned(pdev))
>   		return;
>   
> -	if (!pci_enable_ats(pdev, VTD_PAGE_SHIFT))
> -		info->ats_enabled = 1;
> +	/*
> +	 * pci_enable_ats() should not fail here because earlier checks
> +	 * have already verified support and configuration.
> +	 */
> +	if (WARN_ON(pci_enable_ats(pdev, VTD_PAGE_SHIFT)))
> +		return;
> +
> +	info->ats_enabled = 1;
>   }
>   
>   static void iommu_disable_pci_ats(struct device_domain_info *info)
> @@ -3292,7 +3298,10 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
>   
>   	dev_iommu_priv_set(dev, info);
>   	if (pdev && pci_ats_supported(pdev)) {
> -		pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
> +		ret = pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
> +		if (ret)
> +			goto free;
> +
>   		ret = device_rbtree_insert(iommu, info);
>   		if (ret)
>   			goto free;

Sashiko made a valuable review comment, and I believe it applies here as
well:

[Severity: High]
Since ATS is an optional performance optimization, does failing the
IOMMU probe when pci_prepare_ats() fails break backward compatibility?

This completely prevents devices with buggy ATS capabilities (or VF/PF
STU mismatches) from attaching to the IOMMU.

Could this disable DMA translation entirely for hardware that would
otherwise work correctly without ATS?

Thanks,
baolu

Re: [PATCH v5 6/7] iommu/vt-d: Fail probe on ATS configuration failure

[Pranjal Shrivastava]

On Fri, May 29, 2026 at 02:39:26PM +0800, Baolu Lu wrote:
> On 5/29/26 04:23, Pranjal Shrivastava wrote:
> > Update the Intel VT-d driver to handle ATS configuration and enablement
> > more strictly. Specifically, update the device probe to fail if
> > pci_prepare_ats() returns an error. This ensures that any ATS-capable
> > master reaching the attach phase is guaranteed to have a valid config.
> > 
> > Additionally, update iommu_enable_pci_ats() to WARN() if pci_enable_ats
> > fails. Since earlier checks in the probe phase preclude config-related
> > failures, any failure during hardware enablement is considered a kernel
> > bug.
> > 
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > ---
> >   drivers/iommu/intel/iommu.c | 15 ++++++++++++---
> >   1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index ed6d3a0203f5..f13da16717fe 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -876,8 +876,14 @@ static void iommu_enable_pci_ats(struct device_domain_info *info)
> >   	if (!pci_ats_page_aligned(pdev))
> >   		return;
> > -	if (!pci_enable_ats(pdev, VTD_PAGE_SHIFT))
> > -		info->ats_enabled = 1;
> > +	/*
> > +	 * pci_enable_ats() should not fail here because earlier checks
> > +	 * have already verified support and configuration.
> > +	 */
> > +	if (WARN_ON(pci_enable_ats(pdev, VTD_PAGE_SHIFT)))
> > +		return;
> > +
> > +	info->ats_enabled = 1;
> >   }
> >   static void iommu_disable_pci_ats(struct device_domain_info *info)
> > @@ -3292,7 +3298,10 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
> >   	dev_iommu_priv_set(dev, info);
> >   	if (pdev && pci_ats_supported(pdev)) {
> > -		pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
> > +		ret = pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
> > +		if (ret)
> > +			goto free;
> > +
> >   		ret = device_rbtree_insert(iommu, info);
> >   		if (ret)
> >   			goto free;
> 
> Sashiko made a valuable review comment, and I believe it applies here as
> well:
> 
> [Severity: High]
> Since ATS is an optional performance optimization, does failing the
> IOMMU probe when pci_prepare_ats() fails break backward compatibility?
> 
> This completely prevents devices with buggy ATS capabilities (or VF/PF
> STU mismatches) from attaching to the IOMMU.
> 
> Could this disable DMA translation entirely for hardware that would
> otherwise work correctly without ATS?

Ack. We explored the "not failing probe" strategy in v3, but following
discussion with Jason, we moved toward this "Fail Hard" policy [1]

The rationale is that we ONLY enable ATS IFF ats_is_supported().
If a function claims ATS support via pci_ats_supported() but then fails
in pci_prepare_ats() (e.g., due to an STU mismatch between PF and VF),
it indicates a bug or hardware inconsistency.

Thanks,
Praan

[1] https://lore.kernel.org/all/20260519145947.GK7702@ziepe.ca/

[PATCH v5 7/7] iommu/amd: Fail probe on ATS configuration failure

[Pranjal Shrivastava]

Update the AMD IOMMU driver to handle ATS configuration and enablement
more strictly. Specifically, update the device probe to fail if
pci_prepare_ats() returns an error. This ensures that any ATS-capable
master reaching the attach phase is guaranteed to have a valid config.

Additionally, update pdev_enable_cap_ats() to WARN_ON() if pci_enable_ats
fails. Since earlier checks in the probe phase preclude config-related
failures, any failure during hardware enablement is considered a kernel
bug.

Fix a pre-existing Use-After-Free risk by ensuring iommu_ignore_device()
is called on all probe failures after iommu_init_device().

Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/amd/iommu.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 84cad43dc188..b74c4504bee3 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -570,10 +570,16 @@ static inline int pdev_enable_cap_ats(struct pci_dev *pdev)
 	if (amd_iommu_iotlb_sup &&
 	    (dev_data->flags & AMD_IOMMU_DEVICE_FLAG_ATS_SUP)) {
 		ret = pci_enable_ats(pdev, PAGE_SHIFT);
-		if (!ret) {
-			dev_data->ats_enabled = 1;
-			dev_data->ats_qdep    = pci_ats_queue_depth(pdev);
-		}
+		/*
+		 * pci_enable_ats() should not fail here because earlier checks
+		 * have already verified support and configuration.
+		 */
+		if (WARN_ON(ret))
+			return ret;
+
+		dev_data->ats_enabled = 1;
+		dev_data->ats_qdep    = pci_ats_queue_depth(pdev);
+		ret = 0;
 	}
 
 	return ret;
@@ -2502,10 +2508,22 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 	else
 		dev_data->max_irqs = MAX_IRQS_PER_TABLE_512;
 
-	if (dev_is_pci(dev))
-		pci_prepare_ats(to_pci_dev(dev), PAGE_SHIFT);
+	if (dev_is_pci(dev)) {
+		struct pci_dev *pdev = to_pci_dev(dev);
+
+		if (pci_ats_supported(pdev)) {
+			ret = pci_prepare_ats(pdev, PAGE_SHIFT);
+			if (ret) {
+				iommu_dev = ERR_PTR(ret);
+				goto out_err;
+			}
+		}
+	}
 
 out_err:
+	if (IS_ERR(iommu_dev))
+		iommu_ignore_device(iommu, dev);
+
 	return iommu_dev;
 }
 
-- 
2.54.0.823.g6e5bcc1fc9-goog