Re: [PATCH v4 06/25] drm/xe: Add XE_BO_FLAG_PUT_VM_ASYNC

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Sep 04, 2026 at 12:18:26AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The XE_BO_FLAG_PUT_VM_ASYNC mechanism causes VM reference leaks if xe_bo_put_async() is called when the BO's refcount is greater than 1.
> - [High] Dropping the VM reference synchronously in xe_bo_put_commit causes a Use-After-Free of the shared dma_resv object if TTM defers destruction.
> --
> 
> commit 73ce65401a8053bf565b7b064086594d1dafb49a
> Author: Matthew Brost <matthew.brost@intel.com>
> 
> drm/xe: Add XE_BO_FLAG_PUT_VM_ASYNC
> 
> This commit introduces XE_BO_FLAG_PUT_VM_ASYNC, which indicates that an
> async BO put must also drop an additional reference to the BO's VM. This
> ensures the shared dma-resv object does not disappear before the BO when
> a kernel BO is put asynchronously.
> 
> Link: https://patch.msgid.link/20260228013501.106680-7-matthew.brost@intel.com
> 
> > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> > index dde309821237b..5cde5dff2d481 100644
> > --- a/drivers/gpu/drm/xe/xe_bo.c
> > +++ b/drivers/gpu/drm/xe/xe_bo.c
> > @@ -3989,8 +3989,14 @@ void xe_bo_put_commit(struct llist_head *deferred)
> >  	if (!freed)
> >  		return;
> >  
> > -	llist_for_each_entry_safe(bo, next, freed, freed)
> > +	llist_for_each_entry_safe(bo, next, freed, freed) {
> > +		struct xe_vm *vm = bo->vm;
> > +		bool async = bo->flags & XE_BO_FLAG_PUT_VM_ASYNC;
> > +
> >  		drm_gem_object_free(&bo->ttm.base.refcount);
> > +		if (async)
> > +			xe_vm_put(vm);
> 
> [Severity: High]
> Does this code leak the VM reference if xe_bo_put_async() is called when the
> BO's refcount is greater than 1?
> 
> If xe_bo_put_async() is called while concurrent paths (like show_meminfo or
> xe_bo_evict_all) hold a temporary reference, the BO won't be added to the
> async_list.
> 
> Later, when the temporary reference is dropped via a synchronous xe_bo_put(),
> the BO is destroyed by xe_ttm_bo_destroy(), which only drops bo->vm if
> xe_bo_is_user(bo) is true (which is false for kernel PT BOs). Thus, the VM
> reference could be permanently leaked.
> 
> [Severity: High]
> Can dropping the VM reference synchronously here cause a use-after-free of
> the shared dma_resv object if TTM defers destruction?
> 
> In xe_bo_put_commit(), drm_gem_object_free() drops the GEM reference. If TTM
> concurrently holds an extra reference to the BO (e.g., via
> ttm_bo_get_unless_zero() during an LRU walk), the BO will not be immediately
> destroyed.
> 
> However, xe_bo_put_commit() continues and calls xe_vm_put(vm). If this drops
> the last reference to the VM, the VM and its embedded resv are freed.
> Because the BO is still alive and bo->ttm.base.resv points to the freed
> vm->resv, TTM might later attempt to use the freed reservation object.
> 

I don't think this reasoning is 100% correct, but the reinit of the ref
count is indeed dangerous. I'll rework this part.

Matt

> > +	}
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260903235842.3401722-1-matthew.brost@intel.com?part=6