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 <[email protected]> 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/[email protected]?part=1
