Re: [PATCH v3 1/4] drm/nouveau: unsubscribe the channel-kill event before the fence context

Reviewed-by: Lyude Paul <[email protected]>

On Thu, 2026-08-13 at 01:13 +0200, Marek Czernohous wrote:
> From: Marek Czernohous <[email protected]>
> 
> nouveau_channel_del() tears the fence context down first and only
> drops
> the channel-kill subscription later, in the middle of the nvif object
> teardown:
> 
> 	if (chan->fence)
> 		nouveau_fence(chan->cli->drm)->context_del(chan);
> 	...
> 	nvif_object_dtor(&chan->vram);
> 	nvif_event_dtor(&chan->kill);
> 
> The subscribed handler is nouveau_channel_killed(), which calls
> nouveau_channel_kill() and from there nouveau_fence_context_kill() on
> chan->fence. A kill event delivered in that window takes fctx->lock
> and
> walks fctx->pending on a fence context that context_del() has already
> freed.
> 
> Nothing reaches this below Fermi today, because the subscription is
> gated on FERMI_CHANNEL_GPFIFO and nothing kills a channel there. On
> Fermi and newer the window is real but narrow, since a kill has to
> land
> exactly while the channel is being destroyed. That is reason enough
> on
> its own, which is why this carries a Fixes: tag. The last patch in
> this
> series subscribes Tesla channels as well; nothing kills those today,
> so
> it does not widen the exposure now, but it is the groundwork for a
> recovery path that would, and the ordering is better fixed before
> that
> lands than alongside it.
> 
> Drop the subscription before anything it depends on is torn down.
> 
> Fixes: ea13e5abf807 ("drm/nouveau: signal pending fences when channel
> has been killed")
> Cc: [email protected]
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Marek Czernohous <[email protected]>
> ---
>  drivers/gpu/drm/nouveau/nouveau_chan.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
> b/drivers/gpu/drm/nouveau/nouveau_chan.c
> index 598513f60449..f142f6310596 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_chan.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
> @@ -90,6 +90,14 @@ nouveau_channel_del(struct nouveau_channel
> **pchan)
>  {
>  	struct nouveau_channel *chan = *pchan;
>  	if (chan) {
> +		/*
> +		 * Drop the kill-event subscription first.  Its
> handler
> +		 * dereferences chan->fence, which the fence context
> teardown
> +		 * below frees, so leaving it armed across the
> teardown leaves
> +		 * a window for a use-after-free.
> +		 */
> +		nvif_event_dtor(&chan->kill);
> +
>  		if (chan->fence)
>  			nouveau_fence(chan->cli->drm)-
> >context_del(chan);
>  
> @@ -100,7 +108,6 @@ nouveau_channel_del(struct nouveau_channel
> **pchan)
>  		nvif_object_dtor(&chan->nvsw);
>  		nvif_object_dtor(&chan->gart);
>  		nvif_object_dtor(&chan->vram);
> -		nvif_event_dtor(&chan->kill);
>  		nvif_object_dtor(&chan->user);
>  		nvif_mem_dtor(&chan->mem_userd);
>  		nouveau_vma_del(&chan->sema.vma);