Re: [PATCH v8 4/4] 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 rbtree removal of an unlinked node on error paths in `intel_iommu_probe_device()` empties the device tracking tree.
--

commit edcf0a64984f0847eb81ddb322f530951d47a01a
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 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/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index c3d18cd77d2f1..9242cd9651307 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 unconditional rbtree removal of an unlinked
node on error paths in intel_iommu_probe_device() empties the device
tracking tree.

If a device does not support ATS, it skips the device_rbtree_insert() call
above. If a subsequent failure occurs, such as intel_pasid_alloc_table()
failing, the error path jumps to clear_rbtree and unconditionally calls
device_rbtree_remove():

intel_iommu_probe_device() {
    ...
    clear_rbtree:
        device_rbtree_remove(info);
}

This function directly invokes rb_erase() without verifying whether the node
is actually linked to the tree:

device_rbtree_remove() {
    ...
    rb_erase(&info->node, &iommu->device_rbtree);
}

Because the node is unlinked and completely zero-initialized, the parent
pointer evaluates to NULL. The rbtree implementation treats the node as the
tree's root and overwrites the root with NULL, completely clearing the IOMMU
device tracking tree.

Could we add a check to verify the node is actually linked before calling
rb_erase(), or only jump to clear_rbtree if the node was successfully
inserted?

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