Re: [PATCH 2/2] drm/xe: Convert xe_user_fence to drm_user_fence

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Prematurely dropping the dma_fence reference allows userspace to trigger a kernel WARNING and memory leak via syncobj reset.
--

commit c721250695d79a85665bd6cb949b881db777982f
Author: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

    drm/xe: Convert xe_user_fence to drm_user_fence
    
    Replace the open-coded user fence implementation in xe_sync.c with the
    new common drm_user_fence helper.

> diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
> index 37866768d64c2..e95f50338454c 100644
> --- a/drivers/gpu/drm/xe/xe_sync.c
> +++ b/drivers/gpu/drm/xe/xe_sync.c
[ ... ]
> @@ -282,24 +267,15 @@ void xe_sync_entry_signal(struct xe_sync_entry *sync, struct dma_fence *fence)
>  	} else if (sync->syncobj) {
>  		drm_syncobj_replace_fence(sync->syncobj, fence);
>  	} else if (sync->ufence) {
> -		int err;
> -
>  		drm_syncobj_add_point(sync->ufence_syncobj,
>  				      sync->ufence_chain_fence,
>  				      fence, sync->ufence_timeline_value);
>  		sync->ufence_chain_fence = NULL;
>  
>  		fence = drm_syncobj_fence_get(sync->ufence_syncobj);
> -		user_fence_get(sync->ufence);
> -		err = dma_fence_add_callback(fence, &sync->ufence->cb,
> -					     user_fence_cb);
> -		if (err == -ENOENT) {
> -			kick_ufence(sync->ufence, fence);
> -		} else if (err) {
> +		if (drm_user_fence_add_callback(&sync->ufence->base, fence))
>  			XE_WARN_ON("failed to add user fence");
> -			user_fence_put(sync->ufence);
> -			dma_fence_put(fence);
> -		}
> +		dma_fence_put(fence);
>  	}
>  }

[Severity: High]
Does this code prematurely drop the local fence reference? 

In xe_sync_entry_signal(), we add the user fence callback using 
drm_user_fence_add_callback(), but then immediately call dma_fence_put() 
unconditionally.

If userspace submits a job with a user fence and then subsequently resets 
the syncobj before the fence signals, the syncobj will drop its reference. 
Since we already dropped our local reference here, could the fence be 
destroyed while the callback is still pending? 

This looks like it would trigger a kernel warning from the non-empty callback 
list in dma_fence_release and permanently leak the user fence object since 
the callback is never executed. The previous implementation appeared to avoid 
this by holding the fence reference until the callback actually fired.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827062142.4038272-1-srinivasan.shanmugam@amd.com?part=2