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