Re: [PATCH v9 3/5] drm/xe: have xe_svm_range embed one drm_gpusvm_pages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The framework's cleanup function `drm_gpusvm_fini` violates the newly established contract for `drm_gpusvm_range_remove` by not DMA unmapping pages before removal, creating a potential IOMMU use-after-free vulnerability.
--

commit ce3cfca046dfb562114ff18565b9dc3974af3c6b
Author: Honglei Huang <honghuan@amd.com>
Subject: drm/xe: have xe_svm_range embed one drm_gpusvm_pages

With drm_gpusvm_pages now self contained, make xe stop relying
on the drm_gpusvm_range pages and take responsibility for the page
lifecycle on the driver side.

> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1228,12 +1228,17 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_free_pages);
>   * This function removes the specified GPU SVM range and also removes the parent
>   * GPU SVM notifier if no more ranges remain in the notifier. The caller must
>   * hold a lock to protect range and notifier removal.
> + *
> + * This function does not unmap or free the drm_gpusvm_pages, the driver owns
> + * that lifecycle. The caller must DMA unmap the range's pages before calling
> + * this function, so a range is never removed from the MMU interval tree while
> + * still DMA mapped. Typically the driver calls drm_gpusvm_unmap_pages() first.
> + * And the range_free callback's drm_gpusvm_free_pages() is a final fallback safe
> + * net.
>   */
>  void drm_gpusvm_range_remove(struct drm_gpusvm *gpusvm,
>  			     struct drm_gpusvm_range *range)
>  {

[Severity: High]
Does this new contract break the framework's cleanup function?

Looking at drm_gpusvm_fini() in drm_gpusvm.c, it calls drm_gpusvm_range_remove()
directly on remaining ranges after unregistering the MMU notifier:

		mmu_interval_notifier_remove(&notifier->notifier);
		notifier->flags.removed = true;

		drm_gpusvm_for_each_range_safe(range, __next, notifier, 0,
					       LONG_MAX)
			drm_gpusvm_range_remove(gpusvm, range);

Since drm_gpusvm_range_remove() no longer synchronously unmaps the pages, and
instead relies on the final kref_put() to execute the DMA unmap, could an
elevated refcount on a range indefinitely delay the DMA unmap? 

This would mean the pages remain DMA mapped even though the MMU notifier has
been destroyed. If the OS frees or repurposes these physical pages, the GPU
IOMMU mapping would still point to them, creating a potential use-after-free
regression.

Can we ensure pages are unmapped here, or is there another way to close this
window?

> -	unsigned long npages = npages_in_range(drm_gpusvm_range_start(range),
> -					       drm_gpusvm_range_end(range));
>  	struct drm_gpusvm_notifier *notifier;

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