Re: [PATCH v2] drm/nouveau: Simplify nouveau_cli_work

[sashiko-bot@kernel.org]

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 <tvrtko.ursulin@igalia.com>

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/20260612172227.54893-1-tvrtko.ursulin@igalia.com?part=1