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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unconditional call to `device_rbtree_remove()` for an uninserted `rb_node` corrupts the IOMMU device rbtree.
--

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

iommu/vt-d: Fail probe on ATS configuration failure

Update the Intel VT-d driver to handle ATS configuration strictly by failing
the device probe if pci_prepare_ats() returns an error. Also, add a WARN_ON
in iommu_enable_pci_ats() to catch enablement failures.

> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 4d0e65bc131d7..22308e4911e1f 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
[ ... ]
> @@ -3288,7 +3294,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;

[Severity: High]
This is a pre-existing issue, but I wanted to ask about the error handling
paths in this function. If a device does not support ATS or isn't a PCI
device, device_rbtree_insert() is skipped here.

However, if a subsequent initialization step fails, such as allocating the
PASID table:

intel_iommu_probe_device() {
	...
	if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev)) {
		ret = intel_pasid_alloc_table(dev);
		if (ret) {
			dev_err(dev, "PASID table allocation failed\n");
			goto clear_rbtree;
		}
	...
clear_rbtree:
	device_rbtree_remove(info);
free:
	kfree(info);
	...
}

Does this unconditionally call device_rbtree_remove() on a zero-initialized
and uninserted rb_node?

If rb_erase() is called on this uninserted node, could it treat the zeroed
node as the root and overwrite the actual rbtree root with NULL, corrupting
the IOMMU device tracking tree and dropping previously probed ATS devices?

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