linux-kernel - Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <af03b541-0b69-4b3d-b498-b68e0beb3dcb@amd.com>
Date: Thu, 22 May 2025 14:06:05 +0200
From: Christian König <christian.koenig@....com>
To: Philipp Stanner <phasta@...nel.org>, Lyude Paul <lyude@...hat.com>,
 Danilo Krummrich <dakr@...nel.org>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Sumit Semwal <sumit.semwal@...aro.org>
Cc: dri-devel@...ts.freedesktop.org, nouveau@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, linux-media@...r.kernel.org
Subject: Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence
 context

On 5/22/25 13:25, Philipp Stanner wrote:
> dma_fence_is_signaled_locked(), which is used in
> nouveau_fence_context_kill(), can signal fences below the surface
> through a callback.
> 
> There is neither need for nor use in doing that when killing a fence
> context.
> 
> Replace dma_fence_is_signaled_locked() with __dma_fence_is_signaled(), a
> function which only checks, never signals.

That is not a good approach.

Having the __dma_fence_is_signaled() means that other would be allowed to call it as well.

But nouveau can do that here only because it knows that the fence was issued by nouveau.

What nouveau can to is to test the signaled flag directly, but that's what you try to avoid as well.

Regards,
Christian.

> 
> Signed-off-by: Philipp Stanner <phasta@...nel.org>
> ---
>  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
> index d5654e26d5bc..993b3dcb5db0 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> @@ -88,7 +88,7 @@ nouveau_fence_context_kill(struct nouveau_fence_chan *fctx, int error)
>  
>  	spin_lock_irqsave(&fctx->lock, flags);
>  	list_for_each_entry_safe(fence, tmp, &fctx->pending, head) {
> -		if (error && !dma_fence_is_signaled_locked(&fence->base))
> +		if (error && !__dma_fence_is_signaled(&fence->base))
>  			dma_fence_set_error(&fence->base, error);
>  
>  		if (nouveau_fence_signal(fence))