Re: [PATCH] drm/xe: Tear down kernel mappings in release_notify

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: use-after-free in __xe_bo_vunmap() after xe_bo_shrink_purge()
--

commit e49e34413e8a801c5062d0f07639d200170d532c
Author: Arvind Yadav <arvind.yadav@intel.com>

drm/xe: Tear down kernel mappings in release_notify

This commit moves the __xe_bo_vunmap() call from xe_gem_object_free() to
xe_ttm_bo_release_notify() so that kernel mappings are torn down when the
TTM BO reference count reaches zero rather than when the GEM object
reference count drops to zero.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 42a6aa1a2e4a0..b7a982a9d81ac 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1866,6 +1866,12 @@ static void xe_ttm_bo_release_notify(struct ttm_buffer_object *ttm_bo)
>  	bo = ttm_to_xe_bo(ttm_bo);
>  	xe_assert(xe_bo_device(bo), !(bo->created && kref_read(&ttm_bo->base.refcount)));
>  
> +	/*
> +	 * The TTM BO refcount is zero, so eviction and swapout cannot
> +	 * take a reference. Unmap before TTM frees the backing store.
> +	 */
> +	__xe_bo_vunmap(bo);
> +
>  	if (!xe_ttm_bo_lock_in_destructor(ttm_bo))
>  		return;
>  
> @@ -2056,7 +2062,6 @@ static void xe_gem_object_free(struct drm_gem_object *obj)
>  	 * driver ttm callbacks is allowed to use the ttm_buffer_object
>  	 * refcount directly if needed.
>  	 */
> -	__xe_bo_vunmap(gem_to_xe_bo(obj));
>  	ttm_bo_fini(container_of(obj, struct ttm_buffer_object, base));
>  }

[Severity: High]
Does deferring __xe_bo_vunmap() until xe_ttm_bo_release_notify() cause a
use-after-free and premature backing store free if the shrinker purges a BO
after its GEM refcount reaches zero?

When a kernel-mapped BO in XE_PL_TT (such as one created via
xe_bo_create_pin_map() or xe_bo_create_pin_map_at_aligned()) is unpinned and
its last GEM reference is dropped, bo->vmap and bo->kmap now remain active
while xe_gem_object_free() calls ttm_bo_fini().

If the shrinker grabs a TTM BO reference before ttm_bo_fini() drops the last
kref, xe_bo_shrink() sees that the GEM refcount is already zero and calls
xe_bo_shrink_purge() directly:

drivers/gpu/drm/xe/xe_bo.c:xe_bo_shrink() {
	...
	if (!xe_bo_is_xe_bo(bo) || !xe_bo_get_unless_zero(xe_bo))
		return xe_bo_shrink_purge(ctx, bo, scanned);
}

Unlike xe_bo_move(), xe_bo_shrink_purge() does not call xe_bo_move_notify()
or __xe_bo_vunmap():

drivers/gpu/drm/xe/xe_bo.c:xe_bo_shrink_purge() {
	...
	if (bo->resource->mem_type != XE_PL_SYSTEM) {
		...
		xe_tt_unmap_sg(xe, bo->ttm);
		ttm_bo_move_null(bo, new_resource);
	}

	*scanned += bo->ttm->num_pages;
	lret = ttm_bo_shrink(ctx, bo, (struct ttm_bo_shrink_flags)
			     {.purge = true,
			      .writeback = false,
			      .allow_move = false});
}

This purges the BO via ttm_bo_shrink() -> ttm_tt_backup() ->
ttm_pool_backup(), which resets the page caching attributes via
set_pages_array_wb() and frees the pages in tt->pages while bo->kmap is
still mapped.

When the shrinker later drops its TTM BO reference,
xe_ttm_bo_release_notify() -> __xe_bo_vunmap() -> ttm_bo_kunmap() will run
and call kunmap(map->page) on an already-freed struct page, or
vunmap(map->virtual) after the underlying physical pages were already freed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102082626.747462-1-arvind.yadav@intel.com?part=1