Re: [PATCH V3 3/3] accel/amdxdna: Fix iommu domain lifetime race during device removal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Delaying iommu_detach_group() to the DRM release action prevents driver rebind while the DRM device is kept open by userspace.
--

commit 873614c426570f83392d242f0aa4d39f76d04bd7
Author: Lizhi Hou <lizhi.hou@amd.com>

accel/amdxdna: Fix iommu domain lifetime race during device removal

This commit fixes a use-after-free race during device removal by moving
the freeing of the iommu domain to a drm managed release action. This
ensures the domain lifetime is maintained by the DRM subsystem until all
managed resources are released.

> diff --git a/drivers/accel/amdxdna/amdxdna_iommu.c b/drivers/accel/amdxdna/amdxdna_iommu.c
> index eff00131d0f80..4f245b969eef1 100644
> --- a/drivers/accel/amdxdna/amdxdna_iommu.c
> +++ b/drivers/accel/amdxdna/amdxdna_iommu.c

[ ... ]

> @@ -153,10 +154,30 @@ void amdxdna_iommu_free(struct amdxdna_dev *xdna, size_t size,
>  	free_pages((unsigned long)cpu_addr, get_order(size));
>  }
>  
> +static void amdxdna_cleanup_force_iova(struct drm_device *dev, void *res)
> +{
> +	struct amdxdna_dev *xdna = to_xdna_dev(dev);
> +
> +	if (xdna->domain) {
> +		iommu_detach_group(xdna->domain, xdna->group);

[Severity: High]
Does delaying iommu_detach_group() to the drm managed release action
prevent driver rebind if userspace holds the DRM file descriptor open?

If a user holds the fd open after the device is unbound via sysfs, the
hardware IOMMU group will stay attached to the domain until the file is
finally closed. If the device is subsequently re-bound, or bound to
vfio-pci, iommu_attach_group() in the new probe would return -EBUSY
because the group is already attached.

Should iommu_detach_group() execute synchronously during amdxdna_remove(),
while only deferring iommu_domain_free() and the IOVA state to this release
action?

> +		put_iova_domain(&xdna->iovad);
> +		iova_cache_put();
> +		iommu_domain_free(xdna->domain);
> +	}
> +
> +	iommu_group_put(xdna->group);
> +}
> +
> +void amdxdna_iommu_fini(struct amdxdna_dev *xdna)
> +{
> +	if (xdna->group && !xdna->domain)
> +		iommu_group_put(xdna->group);
> +}

[Severity: High]
Because amdxdna_iommu_fini() now skips putting the IOMMU group when a
domain exists, does this intentionally defer releasing the group until the
drmm release action runs?

This contributes to the same rebind failure mentioned above, as the
resource is locked for the duration of the open userspace file descriptor.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611055150.3070216-1-lizhi.hou@amd.com?part=3