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

[Baolu Lu <baolu.lu@linux.intel.com>]

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