Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] Replacing the locked fence check (`nouveau_cli_work_ready`) with the lockless `dma_fence_is_signaled()` removes critical synchronization with `dma_fence_signal()`, introducing a race condition that leads to a Use-After-Free of `struct nouveau_cli_work`. --
commit f14e589461298a55cf7c0a8d50dd778a913dd4fb Author: Tvrtko Ursulin <[email protected]> drm/nouveau: Simplify nouveau_cli_work This commit simplifies nouveau_cli_work by removing the nouveau_cli_work_ready helper and replacing it with a direct call to dma_fence_is_signaled. It also changes the fence reference release to happen after the work callback is invoked. > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c > b/drivers/gpu/drm/nouveau/nouveau_drm.c > index e16f59b00f6fa..a6432eede6137 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c [ ... ] > @@ -176,9 +160,12 @@ nouveau_cli_work(struct work_struct *w) > struct nouveau_cli_work *work, *wtmp; > mutex_lock(&cli->lock); > list_for_each_entry_safe(work, wtmp, &cli->worker, head) { > - if (!work->fence || nouveau_cli_work_ready(work->fence)) { > + struct dma_fence *fence = work->fence; > + > + if (!fence || dma_fence_is_signaled(fence)) { [Severity: Critical] Can this lockless check introduce a race condition leading to a use-after-free of the work structure? The original nouveau_cli_work_ready() acquired the fence spinlock before checking if the fence was signaled, synchronizing with dma_fence_signal(). By using the lockless dma_fence_is_signaled(), this code can observe the signaled bit while dma_fence_signal() is still iterating over its local callback list. If work->func(work) then deallocates the work structure (for instance, via kfree in nouveau_gem_object_delete_work()), could the signaling CPU subsequently trigger a use-after-free when it invokes the callback (nouveau_cli_work_fence)? > list_del(&work->head); > work->func(work); > + dma_fence_put(fence); > } > } > mutex_unlock(&cli->lock); -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
