Re: [PATCH v4 5/5] iommu/amd: Fail probe on ATS configuration failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Critical] The error path for pci_prepare_ats() in amd_iommu_probe_device() fails to revert previously initialized IOMMU state, leading to a potential Use-After-Free.
- [High] Failing the IOMMU device probe when pci_prepare_ats() fails breaks backward compatibility for devices that could function perfectly without ATS.

Pre-existing issues:
- [Critical] Pre-existing missing cleanup on error path when IOMMU page tables are disabled causes a Use-After-Free.
--

commit c40e9d6d303549c37f626ed11947fa01974222f8
Author: Pranjal Shrivastava <praan@google.com>

iommu/amd: Fail probe on ATS configuration failure

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.

> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 84cad43dc188c..1dddb08e7b224 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
[ ... ]
> @@ -2502,8 +2508,17 @@ 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;

[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?

[Severity: Critical]
If pci_prepare_ats() fails, we return ERR_PTR(ret) without calling
iommu_ignore_device(). Could this lead to a Use-After-Free regression?

Since iommu_init_device() has already globally linked dev_data and populated
the hardware dev_table and rlookup_table with dev_data->dev pointing to the
PCI device struct, the IOMMU core receives an error and considers the device
unbound. It will not invoke release_device() upon removal.

When the physical device is later removed and freed by the PCI subsystem,
dev_data->dev becomes a dangling pointer.

If a spurious IOMMU fault or event occurs for that device ID, the interrupt
handler (e.g., amd_iommu_int_thread) will retrieve dev_data and dereference
the freed pointer (e.g., via dev_name(dev_data->dev) in dump_dte_entry()),
causing a Use-After-Free regression and system crash.

> +			}
> +		}
> +	}
>  
>  out_err:

[Severity: Critical]
This is a pre-existing issue, but does the same Use-After-Free regression
occur when amd_iommu_pgtable == PD_MODE_NONE evaluates to true?

Similar to the newly introduced ATS error path, it jumps to out_err and
returns ERR_PTR(-ENODEV) without calling iommu_ignore_device().

This leaves dev_data globally linked with a populated hardware dev_table and
rlookup_table containing a dangling dev_data->dev pointer. Upon device
removal and a subsequent spurious IOMMU event, could this cause a
Use-After-Free regression for the exact same reasons?

>  	return iommu_dev;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525184347.4059549-1-praan@google.com?part=5