[PATCH 1/3] drm/nouveau: destroy the fence event before cancelling its work

From: Marek Czernohous <[email protected]>

nouveau_fence_context_del() cancels the uevent work first and only tears
the event down afterwards:

	cancel_work_sync(&fctx->uevent_work);
	nouveau_fence_context_kill(fctx, 0);
	nvif_event_dtor(&fctx->event);

Between the cancel and the dtor the event is still armed, and
nouveau_fence_wait_uevent_handler() queues the work unconditionally:

	schedule_work(&fctx->uevent_work);
	return NVIF_EVENT_KEEP;

So a non-stall interrupt arriving in that window re-arms the work that
was just cancelled. The callers free the context immediately afterwards,
for example nv84_fence_context_del():

	nouveau_fence_context_del(&fctx->base);
	chan->fence = NULL;
	nouveau_fence_context_free(&fctx->base);

nouveau_fence_uevent_work() then runs against freed memory, taking
fctx->lock and walking fctx->pending.

Only chips from G84 on can reach this at all: nouveau_fence_context_new()
returns before nvif_event_ctor() when priv->uevent is clear, and
nv84_fence_create() is the only place that sets it.
nv84_fence_context_del() is the context_del for all of those, because
nvc0_fence_create() and gv100_fence_create() build on nv84_fence_create()
and override only context_new.

Drop the event first, so no further work can be queued, and only then
drain what is already queued.

nouveau_fence_context_kill() keeps its place after the drain. It can
still touch the event: nouveau_fence_signal() returns true when a fence
that had enable_signaling() called on it is signalled and
fctx->notify_ref drops to zero, and the loop then calls
nvif_event_block() once. That call runs in the same thread just after
the dtor, where nvif_event_constructed() is false and it is a no-op.
Blocking an event that no longer exists would be pointless anyway.

The reordering does open one window, so it is worth saying what closes
it. A fence holder that reaches nouveau_fence_enable_signaling()
between the dtor and the kill gets a silent no-op from
nvif_event_allow(), so that fence will not be woken by a non-stall
interrupt any more. It does not have to be: the kill runs immediately
afterwards, signals every fence on fctx->pending under fctx->lock and
sets fctx->killed, after which nouveau_fence_emit() refuses further
work with -ENODEV.

Reported-by: sashiko-bot <[email protected]>
Link: https://sashiko.dev/#/patchset/[email protected]?part=1
Fixes: 39126abc5e20 ("nouveau: offload fence uevents work to workqueue")
Cc: [email protected]
Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <[email protected]>
---
 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 edbe9e08ba0f..4a3698dc2cd1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -96,9 +96,9 @@ nouveau_fence_context_kill(struct nouveau_fence_chan *fctx, int error)
 void
 nouveau_fence_context_del(struct nouveau_fence_chan *fctx)
 {
+	nvif_event_dtor(&fctx->event);
 	cancel_work_sync(&fctx->uevent_work);
 	nouveau_fence_context_kill(fctx, 0);
-	nvif_event_dtor(&fctx->event);
 	fctx->dead = 1;
 
 	/*
-- 
2.54.0