[PATCH v3 0/4] drm/nouveau: channel-kill event ordering fixes, and lower the gate to NV50

[PATCH v3 0/4] drm/nouveau: channel-kill event ordering fixes, and lower the gate to NV50

[Marek Czernohous]

From: Marek Czernohous <marek@czernohous.de>

This is v3 of the nv04 FIFO series, cut down to the four patches that are
ready. v2 is here:

  https://lore.kernel.org/nouveau/20260806085228.1848994-1-mczernohous@gmail.com/

The Sashiko review bot flagged three issues on that posting. It was right on
all three. Two are fixed by the new patches at the head of this series; the
third was in the Tesla recovery patch, which I have dropped from this posting.
More on that below, because the reason is not the bot's finding.

1/4 The channel-kill subscription outlives the fence context in
    nouveau_channel_del(): the context is freed first and the event is only
    dropped later, among the nvif object teardown. A kill delivered in that
    window walks a freed fence context.

2/4 The subscription is armed before the fence context exists.
    nouveau_channel_init() subscribes right after mapping userd and creates
    the context at the end of the same function, and the backends publish
    chan->fence from kzalloc() before nouveau_fence_context_new() initialises
    its lock and pending list. The NULL check in nouveau_channel_kill() does
    not cover that: the pointer is non-NULL and unusable.

Both are pre-existing. Both are unreachable below Fermi today, and on Fermi
and newer the windows are narrow, but they are real, so 1/4 and 2/4 carry
Fixes: and Cc: stable and stand on their own. I should have carried them from
the start. The v2 cover letter even described the teardown ordering and
dismissed it as "most of that window is harmless". That judgement was wrong,
and I appreciate the catch.

3/4 and 4/4 are unchanged in code apart from the rebase and one joined format
string. Both commit messages have been corrected: 3/4 no longer implies a chip
gate it does not have and no longer claims a before and after I cannot show,
and 4/4 no longer says the fences time out, because they cannot.

3/4 downgrades one specific CACHE_ERROR to debug level. Mesa's NV50 bind
probe writes a known method and data pattern that the hardware rejects,
repeatedly over the life of a session, and nouveau has been reporting it at
error level ever since. It changes no behaviour beyond the log level and is
unrelated to the two fixes above; it has simply travelled with them since
v2.

4/4 lowers the channel-kill subscription from FERMI_CHANNEL_GPFIFO to
NV50_CHANNEL_GPFIFO. On its own it changes no observable behaviour, because
nothing kills a channel on Tesla today: the only nvkm_chan_error() callers are
the Fermi and newer recovery paths. It is worth having anyway. Leaving the
ERRORED event delivered into an empty notifier list is a latent dma-fence
contract violation, and it is exactly the trap the recovery work would fall
into. Removing it before anything can fall in is the right order.

What is not in this posting, and why

  v2 carried a third patch, the recovery path for Tesla cache_error and
  dma_pusher faults, and the five-patch v3 I had prepared carried it as 5/5.
  It is not in this posting, and the bot's third finding is not the reason:
  that one was a real bug of mine and is fixed.

  I ran an adversarial review over the whole series before sending, and it
  found two defects in that patch that the bot had not flagged:

  - nv04_fifo_recover() has no chip-family guard, but nv04_fifo_intr is .intr
    for nv04, nv10, nv17, nv40, nv50, g84 and g98. On NV04 to NV40 the
    escalation would therefore fire while 4/4 deliberately installs no
    subscriber, producing exactly the dead-letter hang that 4/4 exists to
    prevent. The series would have contradicted its own reasoning, in the same
    diff.

  - The recovery path pulls the drm_device out of drvdata on every fault,
    while nouveau never clears drvdata and the only cancel_work_sync() for the
    wedge work runs in nvkm_fifo_dtor(), that is after nouveau_drm_device_del()
    has already kfree()d the nouveau_drm. A queued work reads freed memory.

  The first is a one-line guard. The second wants the wedge policy moved into
  the DRM layer rather than patched where it is, which also removes an nvkm to
  DRM layering violation I should not have introduced. That is a rework, not a
  fixup, so it goes into its own posting once it has been rebuilt and soaked.
  Sending it now would only waste reviewer time.

Testing

  Reference hardware: Apple Mac mini Late 2009, MCP79 / GeForce 9400M (NVAC),
  Core 2 Duo, Wayland (labwc).

  Build. Unlike v2, this series is built against the stated base commit: the
  nouveau module compiles and links with zero warnings and zero errors, with
  the new format strings and the lowered class gate in place. checkpatch.pl
  --strict is clean on all four patches.

  Soak. Code equivalent to 1/4, 3/4 and 4/4 has been running on that machine
  since 2026-07-25, and equivalent to 2/4 since 2026-08-06. The first spans
  the kernel bumps 7.1.5, 7.1.6, 7.1.7 and 7.1.8, the second only 7.1.7 and
  7.1.8. Since every channel now subscribes to the kill event, the interesting
  long-run question is channel churn, so the soak is ordinary daily driving
  with a browser and GL applications rather than a synthetic loop. No
  regression has shown up in that time.

  What the soak does not show, stated plainly: that tree carries local patches
  this series does not, including a cap on the plane-fence wait in the
  nonblocking commit tail. So the soak says these changes do not misbehave in
  daily use. It is not an independent demonstration of the failure modes
  described above.

  1/4 and 2/4 are ordering fixes for windows I have not managed to hit
  deliberately on this hardware; the reasoning is from the source, and the
  bot's reading of it agrees. I would rather say that plainly than claim a
  reproduction I do not have.

  3/4 filters a benign CACHE_ERROR that Mesa's NV50 bind probe triggers. The
  logs behind it are from a second, independent MCP79/MCP7A machine, not
  mine: 99 occurrences across three logs and two kernel versions, under Xorg
  and Wayland, with kwin and plasmashell named as the faulting clients. They
  were shared with me by the tester who reported the original NVAC issue. I
  have not named them and have added no Reported-by or Tested-by: we have
  not been in touch since then, so I have nobody's consent to point at. On
  my own machine the filter went in before the persistent logging did, so I
  cannot show a clean before and after from here.

AI assistance

  Lyude asked on the v1 thread whether these patches were written by a human
  and pointed at Documentation/process/coding-assistants.rst. The answer,
  repeated here for the archive: this work is AI assisted. I use Claude
  (claude-opus-4-7 for v1, claude-opus-5 since) as a coding and analysis
  assistant. Every patch carries an Assisted-by trailer accordingly, and no
  Signed-off-by is added by the tool.

  Nature of the assistance, so you can calibrate your review: the assistant did
  most of the code archaeology and drafting. I described symptoms, asked for
  the mechanism to be traced in the source rather than guessed, and asked for
  each claim to be backed by a file and line. The measurements quoted in
  earlier postings are from the machine, not model output. The assistant also
  reviewed its own earlier work adversarially, which is how the two defects in
  the dropped patch were found, and how several unsupported claims in the v2
  cover letter were caught before this posting. I reviewed the result, I
  understand the code, and I take responsibility for it.

Changes since v2

  - new 1/4: unsubscribe the channel-kill event before the fence context
  - new 2/4: subscribe to the channel-kill event after the fence context
  - both found by the Sashiko review bot on the v2 posting, confirmed against
    the source before acting on them, and now carrying Fixes: and Cc: stable
  - dropped the Tesla recovery patch, see above
  - 3/4 and 4/4 unchanged in code apart from the rebase and one joined format
    string, both commit messages corrected for accuracy
  - the series is now built against the base commit

Marek Czernohous (4):
  drm/nouveau: unsubscribe the channel-kill event before the fence
    context
  drm/nouveau: subscribe to the channel-kill event after the fence
    context
  drm/nouveau/fifo/nv04: filter benign CACHE_ERROR from Mesa NV50 bind
    probe
  drm/nouveau: subscribe to channel-kill events on NV50 and newer

 drivers/gpu/drm/nouveau/nouveau_chan.c        | 64 ++++++++++++-------
 .../gpu/drm/nouveau/nvkm/engine/fifo/nv04.c   | 26 ++++++--
 2 files changed, 61 insertions(+), 29 deletions(-)


base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86
-- 
2.54.0

[PATCH v3 0/4] drm/nouveau: channel-kill event ordering fixes, and lower the gate to NV50

[Marek Czernohous]

From: Marek Czernohous <marek@czernohous.de>

This is v3 of the nv04 FIFO series, cut down to the four patches that are
ready. v2 is here:

  https://lore.kernel.org/nouveau/20260806085228.1848994-1-mczernohous@gmail.com/

The Sashiko review bot flagged three issues on that posting. It was right on
all three. Two are fixed by the new patches at the head of this series; the
third was in the Tesla recovery patch, which I have dropped from this posting.
More on that below, because the reason is not the bot's finding.

1/4 The channel-kill subscription outlives the fence context in
    nouveau_channel_del(): the context is freed first and the event is only
    dropped later, among the nvif object teardown. A kill delivered in that
    window walks a freed fence context.

2/4 The subscription is armed before the fence context exists.
    nouveau_channel_init() subscribes right after mapping userd and creates
    the context at the end of the same function, and the backends publish
    chan->fence from kzalloc() before nouveau_fence_context_new() initialises
    its lock and pending list. The NULL check in nouveau_channel_kill() does
    not cover that: the pointer is non-NULL and unusable.

Both are pre-existing. Both are unreachable below Fermi today, and on Fermi
and newer the windows are narrow, but they are real, so 1/4 and 2/4 carry
Fixes: and Cc: stable and stand on their own. I should have carried them from
the start. The v2 cover letter even described the teardown ordering and
dismissed it as "most of that window is harmless". That judgement was wrong,
and I appreciate the catch.

3/4 and 4/4 are unchanged in code apart from the rebase and one joined format
string. Both commit messages have been corrected: 3/4 no longer implies a chip
gate it does not have and no longer claims a before and after I cannot show,
and 4/4 no longer says the fences time out, because they cannot.

3/4 downgrades one specific CACHE_ERROR to debug level. Mesa's NV50 bind
probe writes a known method and data pattern that the hardware rejects,
repeatedly over the life of a session, and nouveau has been reporting it at
error level ever since. It changes no behaviour beyond the log level and is
unrelated to the two fixes above; it has simply travelled with them since
v2.

4/4 lowers the channel-kill subscription from FERMI_CHANNEL_GPFIFO to
NV50_CHANNEL_GPFIFO. On its own it changes no observable behaviour, because
nothing kills a channel on Tesla today: the only nvkm_chan_error() callers are
the Fermi and newer recovery paths. It is worth having anyway. Leaving the
ERRORED event delivered into an empty notifier list is a latent dma-fence
contract violation, and it is exactly the trap the recovery work would fall
into. Removing it before anything can fall in is the right order.

What is not in this posting, and why

  v2 carried a third patch, the recovery path for Tesla cache_error and
  dma_pusher faults, and the five-patch v3 I had prepared carried it as 5/5.
  It is not in this posting, and the bot's third finding is not the reason:
  that one was a real bug of mine and is fixed.

  I ran an adversarial review over the whole series before sending, and it
  found two defects in that patch that the bot had not flagged:

  - nv04_fifo_recover() has no chip-family guard, but nv04_fifo_intr is .intr
    for nv04, nv10, nv17, nv40, nv50, g84 and g98. On NV04 to NV40 the
    escalation would therefore fire while 4/4 deliberately installs no
    subscriber, producing exactly the dead-letter hang that 4/4 exists to
    prevent. The series would have contradicted its own reasoning, in the same
    diff.

  - The recovery path pulls the drm_device out of drvdata on every fault,
    while nouveau never clears drvdata and the only cancel_work_sync() for the
    wedge work runs in nvkm_fifo_dtor(), that is after nouveau_drm_device_del()
    has already kfree()d the nouveau_drm. A queued work reads freed memory.

  The first is a one-line guard. The second wants the wedge policy moved into
  the DRM layer rather than patched where it is, which also removes an nvkm to
  DRM layering violation I should not have introduced. That is a rework, not a
  fixup, so it goes into its own posting once it has been rebuilt and soaked.
  Sending it now would only waste reviewer time.

Testing

  Reference hardware: Apple Mac mini Late 2009, MCP79 / GeForce 9400M (NVAC),
  Core 2 Duo, Wayland (labwc).

  Build. Unlike v2, this series is built against the stated base commit: the
  nouveau module compiles and links with zero warnings and zero errors, with
  the new format strings and the lowered class gate in place. checkpatch.pl
  --strict is clean on all four patches.

  Soak. Code equivalent to 1/4, 3/4 and 4/4 has been running on that machine
  since 2026-07-25, and equivalent to 2/4 since 2026-08-06. The first spans
  the kernel bumps 7.1.5, 7.1.6, 7.1.7 and 7.1.8, the second only 7.1.7 and
  7.1.8. Since every channel now subscribes to the kill event, the interesting
  long-run question is channel churn, so the soak is ordinary daily driving
  with a browser and GL applications rather than a synthetic loop. No
  regression has shown up in that time.

  What the soak does not show, stated plainly: that tree carries local patches
  this series does not, including a cap on the plane-fence wait in the
  nonblocking commit tail. So the soak says these changes do not misbehave in
  daily use. It is not an independent demonstration of the failure modes
  described above.

  1/4 and 2/4 are ordering fixes for windows I have not managed to hit
  deliberately on this hardware; the reasoning is from the source, and the
  bot's reading of it agrees. I would rather say that plainly than claim a
  reproduction I do not have.

  3/4 filters a benign CACHE_ERROR that Mesa's NV50 bind probe triggers. The
  logs behind it are from a second, independent MCP79/MCP7A machine, not
  mine: 99 occurrences across three logs and two kernel versions, under Xorg
  and Wayland, with kwin and plasmashell named as the faulting clients. They
  were shared with me by the tester who reported the original NVAC issue. I
  have not named them and have added no Reported-by or Tested-by: we have
  not been in touch since then, so I have nobody's consent to point at. On
  my own machine the filter went in before the persistent logging did, so I
  cannot show a clean before and after from here.

AI assistance

  Lyude asked on the v1 thread whether these patches were written by a human
  and pointed at Documentation/process/coding-assistants.rst. The answer,
  repeated here for the archive: this work is AI assisted. I use Claude
  (claude-opus-4-7 for v1, claude-opus-5 since) as a coding and analysis
  assistant. Every patch carries an Assisted-by trailer accordingly, and no
  Signed-off-by is added by the tool.

  Nature of the assistance, so you can calibrate your review: the assistant did
  most of the code archaeology and drafting. I described symptoms, asked for
  the mechanism to be traced in the source rather than guessed, and asked for
  each claim to be backed by a file and line. The measurements quoted in
  earlier postings are from the machine, not model output. The assistant also
  reviewed its own earlier work adversarially, which is how the two defects in
  the dropped patch were found, and how several unsupported claims in the v2
  cover letter were caught before this posting. I reviewed the result, I
  understand the code, and I take responsibility for it.

Changes since v2

  - new 1/4: unsubscribe the channel-kill event before the fence context
  - new 2/4: subscribe to the channel-kill event after the fence context
  - both found by the Sashiko review bot on the v2 posting, confirmed against
    the source before acting on them, and now carrying Fixes: and Cc: stable
  - dropped the Tesla recovery patch, see above
  - 3/4 and 4/4 unchanged in code apart from the rebase and one joined format
    string, both commit messages corrected for accuracy
  - the series is now built against the base commit

Marek Czernohous (4):
  drm/nouveau: unsubscribe the channel-kill event before the fence
    context
  drm/nouveau: subscribe to the channel-kill event after the fence
    context
  drm/nouveau/fifo/nv04: filter benign CACHE_ERROR from Mesa NV50 bind
    probe
  drm/nouveau: subscribe to channel-kill events on NV50 and newer

 drivers/gpu/drm/nouveau/nouveau_chan.c        | 64 ++++++++++++-------
 .../gpu/drm/nouveau/nvkm/engine/fifo/nv04.c   | 26 ++++++--
 2 files changed, 61 insertions(+), 29 deletions(-)


base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86
-- 
2.54.0

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

[Marek Czernohous]

From: Marek Czernohous <marek@czernohous.de>

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: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <marek@czernohous.de>
---
 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);
-- 
2.54.0

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

[Marek Czernohous]

From: Marek Czernohous <marek@czernohous.de>

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: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <marek@czernohous.de>
---
 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);
-- 
2.54.0

[PATCH v3 2/4] drm/nouveau: subscribe to the channel-kill event after the fence context

[Marek Czernohous]

From: Marek Czernohous <marek@czernohous.de>

nouveau_channel_init() arms the channel-kill subscription early, right
after mapping userd, and only creates the fence context at the very end
of the same function. The handler it installs, nouveau_channel_killed(),
reaches nouveau_fence_context_kill(chan->fence).

The NULL check in nouveau_channel_kill() does not cover the window in
between. The backends publish the pointer before the context is usable:

	fctx = chan->fence = kzalloc_obj(*fctx);
	if (!fctx)
		return -ENOMEM;

	nouveau_fence_context_new(chan, &fctx->base);

and nouveau_fence_context_new() is what runs spin_lock_init(&fctx->lock)
and INIT_LIST_HEAD(&fctx->pending). An event arriving after the
assignment but before that call finds chan->fence non-NULL and unusable:
nouveau_fence_context_kill() takes a lock that was never initialised and
walks a list head whose next pointer is still the NULL left by kzalloc().

Move the subscription behind context_new() so the handler cannot observe
a half-built fence context. The failure path is unchanged in effect: the
caller drops the channel with nouveau_channel_del() either way, which
since the previous patch unsubscribes before freeing the context.

One behavioural change worth naming, and it is not free: a kill
delivered while the channel is still initialising is no longer observed,
because the subscription is not armed yet. That window does not close
here, it moves, and on Fermi and newer it grows by the span between the
old subscription point and context_new(). What changes is what the
window costs. Before, a kill landing in it reached a half-built fence
context; now it is missed, and the channel is left blocked with
chan->killed still 0, so nouveau_channel_idle() and the checks in
nouveau_gem_ioctl_pushbuf() and nouveau_exec_ioctl_exec() keep treating
it as alive. The missed-kill window is not introduced by this patch either:
nvkm_uchan_init() already calls nvkm_chan_allow() and nvkm_chan_insert(),
so the channel is schedulable before nouveau_channel_init() subscribes at
all. Closing it properly means subscribing before the channel becomes
schedulable, which is a bigger change than this fix.

As with the previous patch this is unreachable below Fermi today, and
the last patch in this series lowers the gate to NV50.

Fixes: ea13e5abf807 ("drm/nouveau: signal pending fences when channel has been killed")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <marek@czernohous.de>
---
 drivers/gpu/drm/nouveau/nouveau_chan.c | 55 +++++++++++++++-----------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/nouveau/nouveau_chan.c
index f142f6310596..07b0bd1bc519 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -370,27 +370,6 @@ nouveau_channel_init(struct nouveau_channel *chan, u32 vram, u32 gart)
 	if (ret)
 		return ret;
 
-	if (chan->user.oclass >= FERMI_CHANNEL_GPFIFO) {
-		DEFINE_RAW_FLEX(struct nvif_event_v0, args, data,
-				sizeof(struct nvif_chan_event_v0));
-		struct nvif_chan_event_v0 *host =
-				(struct nvif_chan_event_v0 *)args->data;
-
-		host->version = 0;
-		host->type = NVIF_CHAN_EVENT_V0_KILLED;
-
-		ret = nvif_event_ctor(&chan->user, "abi16ChanKilled", chan->chid,
-				      nouveau_channel_killed, false,
-				      args, __struct_size(args), &chan->kill);
-		if (ret == 0)
-			ret = nvif_event_allow(&chan->kill);
-		if (ret) {
-			NV_ERROR(drm, "Failed to request channel kill "
-				      "notification: %d\n", ret);
-			return ret;
-		}
-	}
-
 	/* allocate dma objects to cover all allowed vram, and gart */
 	if (device->info.family < NV_DEVICE_INFO_V0_FERMI) {
 		if (device->info.family >= NV_DEVICE_INFO_V0_TESLA) {
@@ -494,7 +473,39 @@ nouveau_channel_init(struct nouveau_channel *chan, u32 vram, u32 gart)
 	}
 
 	/* initialise synchronisation */
-	return nouveau_fence(drm)->context_new(chan);
+	ret = nouveau_fence(drm)->context_new(chan);
+	if (ret)
+		return ret;
+
+	/*
+	 * Subscribe to the channel-kill event last.  The handler
+	 * dereferences chan->fence, and the fence context is only complete
+	 * once context_new() has returned: the backends assign chan->fence
+	 * from kzalloc() before nouveau_fence_context_new() initialises the
+	 * lock and the pending list, so an event arriving in between would
+	 * find a non-NULL but unusable context and walk a NULL list head.
+	 */
+	if (chan->user.oclass >= FERMI_CHANNEL_GPFIFO) {
+		DEFINE_RAW_FLEX(struct nvif_event_v0, args, data,
+				sizeof(struct nvif_chan_event_v0));
+		struct nvif_chan_event_v0 *host =
+				(struct nvif_chan_event_v0 *)args->data;
+
+		host->version = 0;
+		host->type = NVIF_CHAN_EVENT_V0_KILLED;
+
+		ret = nvif_event_ctor(&chan->user, "abi16ChanKilled", chan->chid,
+				      nouveau_channel_killed, false,
+				      args, __struct_size(args), &chan->kill);
+		if (ret == 0)
+			ret = nvif_event_allow(&chan->kill);
+		if (ret) {
+			NV_ERROR(drm, "Failed to request channel kill notification: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
 }
 
 int
-- 
2.54.0

[PATCH v3 2/4] drm/nouveau: subscribe to the channel-kill event after the fence context

[Marek Czernohous]

From: Marek Czernohous <marek@czernohous.de>

nouveau_channel_init() arms the channel-kill subscription early, right
after mapping userd, and only creates the fence context at the very end
of the same function. The handler it installs, nouveau_channel_killed(),
reaches nouveau_fence_context_kill(chan->fence).

The NULL check in nouveau_channel_kill() does not cover the window in
between. The backends publish the pointer before the context is usable:

	fctx = chan->fence = kzalloc_obj(*fctx);
	if (!fctx)
		return -ENOMEM;

	nouveau_fence_context_new(chan, &fctx->base);

and nouveau_fence_context_new() is what runs spin_lock_init(&fctx->lock)
and INIT_LIST_HEAD(&fctx->pending). An event arriving after the
assignment but before that call finds chan->fence non-NULL and unusable:
nouveau_fence_context_kill() takes a lock that was never initialised and
walks a list head whose next pointer is still the NULL left by kzalloc().

Move the subscription behind context_new() so the handler cannot observe
a half-built fence context. The failure path is unchanged in effect: the
caller drops the channel with nouveau_channel_del() either way, which
since the previous patch unsubscribes before freeing the context.

One behavioural change worth naming, and it is not free: a kill
delivered while the channel is still initialising is no longer observed,
because the subscription is not armed yet. That window does not close
here, it moves, and on Fermi and newer it grows by the span between the
old subscription point and context_new(). What changes is what the
window costs. Before, a kill landing in it reached a half-built fence
context; now it is missed, and the channel is left blocked with
chan->killed still 0, so nouveau_channel_idle() and the checks in
nouveau_gem_ioctl_pushbuf() and nouveau_exec_ioctl_exec() keep treating
it as alive. The missed-kill window is not introduced by this patch either:
nvkm_uchan_init() already calls nvkm_chan_allow() and nvkm_chan_insert(),
so the channel is schedulable before nouveau_channel_init() subscribes at
all. Closing it properly means subscribing before the channel becomes
schedulable, which is a bigger change than this fix.

As with the previous patch this is unreachable below Fermi today, and
the last patch in this series lowers the gate to NV50.

Fixes: ea13e5abf807 ("drm/nouveau: signal pending fences when channel has been killed")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <marek@czernohous.de>
---
 drivers/gpu/drm/nouveau/nouveau_chan.c | 55 +++++++++++++++-----------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/nouveau/nouveau_chan.c
index f142f6310596..07b0bd1bc519 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -370,27 +370,6 @@ nouveau_channel_init(struct nouveau_channel *chan, u32 vram, u32 gart)
 	if (ret)
 		return ret;
 
-	if (chan->user.oclass >= FERMI_CHANNEL_GPFIFO) {
-		DEFINE_RAW_FLEX(struct nvif_event_v0, args, data,
-				sizeof(struct nvif_chan_event_v0));
-		struct nvif_chan_event_v0 *host =
-				(struct nvif_chan_event_v0 *)args->data;
-
-		host->version = 0;
-		host->type = NVIF_CHAN_EVENT_V0_KILLED;
-
-		ret = nvif_event_ctor(&chan->user, "abi16ChanKilled", chan->chid,
-				      nouveau_channel_killed, false,
-				      args, __struct_size(args), &chan->kill);
-		if (ret == 0)
-			ret = nvif_event_allow(&chan->kill);
-		if (ret) {
-			NV_ERROR(drm, "Failed to request channel kill "
-				      "notification: %d\n", ret);
-			return ret;
-		}
-	}
-
 	/* allocate dma objects to cover all allowed vram, and gart */
 	if (device->info.family < NV_DEVICE_INFO_V0_FERMI) {
 		if (device->info.family >= NV_DEVICE_INFO_V0_TESLA) {
@@ -494,7 +473,39 @@ nouveau_channel_init(struct nouveau_channel *chan, u32 vram, u32 gart)
 	}
 
 	/* initialise synchronisation */
-	return nouveau_fence(drm)->context_new(chan);
+	ret = nouveau_fence(drm)->context_new(chan);
+	if (ret)
+		return ret;
+
+	/*
+	 * Subscribe to the channel-kill event last.  The handler
+	 * dereferences chan->fence, and the fence context is only complete
+	 * once context_new() has returned: the backends assign chan->fence
+	 * from kzalloc() before nouveau_fence_context_new() initialises the
+	 * lock and the pending list, so an event arriving in between would
+	 * find a non-NULL but unusable context and walk a NULL list head.
+	 */
+	if (chan->user.oclass >= FERMI_CHANNEL_GPFIFO) {
+		DEFINE_RAW_FLEX(struct nvif_event_v0, args, data,
+				sizeof(struct nvif_chan_event_v0));
+		struct nvif_chan_event_v0 *host =
+				(struct nvif_chan_event_v0 *)args->data;
+
+		host->version = 0;
+		host->type = NVIF_CHAN_EVENT_V0_KILLED;
+
+		ret = nvif_event_ctor(&chan->user, "abi16ChanKilled", chan->chid,
+				      nouveau_channel_killed, false,
+				      args, __struct_size(args), &chan->kill);
+		if (ret == 0)
+			ret = nvif_event_allow(&chan->kill);
+		if (ret) {
+			NV_ERROR(drm, "Failed to request channel kill notification: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
 }
 
 int
-- 
2.54.0

[PATCH v3 3/4] drm/nouveau/fifo/nv04: filter benign CACHE_ERROR from Mesa NV50 bind probe

[Marek Czernohous]

From: Marek Czernohous <marek@czernohous.de>

The Mesa userspace driver issues a method-0x0060 / data-0xbeef02xx
binding probe that ends up triggering CACHE_ERROR in the PFIFO
interrupt handler. The probe is harmless and recovers cleanly, but it
is reported at error level, so it shows up in dmesg on session start.

Filter that specific pattern down to debug level so dmesg stays clean
while real CACHE_ERROR conditions are still logged at error level.

The test is on the method and data pattern alone, not on the chip
family, so it applies wherever nv04_fifo_intr() is the handler, that is
nv04 through g98. That is deliberate rather than an oversight: a false
positive would need userspace to write exactly 0xbeef02xx to method
0x0060, and the probe itself comes from the shared nouveau Gallium code
rather than from anything NV50 specific. Say so here so the narrower
wording of the subject is not read as a chip gate.

Evidence: 99 occurrences across three logs from a second, independent
MCP79/MCP7A machine running 7.0.10 and 6.12.90, under both Xorg and
Wayland, with kwin and plasmashell named as the faulting clients. On my
own reference machine the filter went in before the persistent kernel
log did, so I cannot show a clean before and after from there.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <marek@czernohous.de>
---
 .../gpu/drm/nouveau/nvkm/engine/fifo/nv04.c   | 26 ++++++++++++++-----
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/nv04.c b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/nv04.c
index c4b8e567d86f..ab144c1bd9da 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/nv04.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/nv04.c
@@ -327,12 +327,26 @@ nv04_fifo_intr_cache_error(struct nvkm_fifo *fifo, u32 chid, u32 get)
 
 	if (!(pull0 & 0x00000100) ||
 	    !nv04_fifo_swmthd(device, chid, mthd, data)) {
-		chan = nvkm_chan_get_chid(&fifo->engine, chid, &flags);
-		nvkm_error(subdev, "CACHE_ERROR - "
-			   "ch %d [%s] subc %d mthd %04x data %08x\n",
-			   chid, chan ? chan->name : "unknown",
-			   (mthd >> 13) & 7, mthd & 0x1ffc, data);
-		nvkm_chan_put(&chan, flags);
+		/*
+		 * Filter the benign Mesa bind probe: mthd 0x0060 with data
+		 * 0xbeef02xx is a harmless userspace probe and does not
+		 * indicate an actual error condition. The test is on the
+		 * method and data pattern alone, so it applies on every
+		 * chip that reaches this handler, not just on Tesla.
+		 * Demote to debug to keep dmesg clean while still catching
+		 * real CACHE_ERROR events.
+		 */
+		if ((mthd & 0x1ffc) == 0x0060 &&
+		    (data & 0xffffff00) == 0xbeef0200) {
+			nvkm_debug(subdev, "CACHE_ERROR - ch %d subc %d mthd %04x data %08x (benign, skipped)\n",
+				   chid, (mthd >> 13) & 7, mthd & 0x1ffc, data);
+		} else {
+			chan = nvkm_chan_get_chid(&fifo->engine, chid, &flags);
+			nvkm_error(subdev, "CACHE_ERROR - ch %d [%s] subc %d mthd %04x data %08x\n",
+				   chid, chan ? chan->name : "unknown",
+				   (mthd >> 13) & 7, mthd & 0x1ffc, data);
+			nvkm_chan_put(&chan, flags);
+		}
 	}
 
 	nvkm_wr32(device, NV04_PFIFO_CACHE1_DMA_PUSH, 0);
-- 
2.54.0

[PATCH v3 3/4] drm/nouveau/fifo/nv04: filter benign CACHE_ERROR from Mesa NV50 bind probe

[Marek Czernohous]

From: Marek Czernohous <marek@czernohous.de>

The Mesa userspace driver issues a method-0x0060 / data-0xbeef02xx
binding probe that ends up triggering CACHE_ERROR in the PFIFO
interrupt handler. The probe is harmless and recovers cleanly, but it
is reported at error level, so it shows up in dmesg on session start.

Filter that specific pattern down to debug level so dmesg stays clean
while real CACHE_ERROR conditions are still logged at error level.

The test is on the method and data pattern alone, not on the chip
family, so it applies wherever nv04_fifo_intr() is the handler, that is
nv04 through g98. That is deliberate rather than an oversight: a false
positive would need userspace to write exactly 0xbeef02xx to method
0x0060, and the probe itself comes from the shared nouveau Gallium code
rather than from anything NV50 specific. Say so here so the narrower
wording of the subject is not read as a chip gate.

Evidence: 99 occurrences across three logs from a second, independent
MCP79/MCP7A machine running 7.0.10 and 6.12.90, under both Xorg and
Wayland, with kwin and plasmashell named as the faulting clients. On my
own reference machine the filter went in before the persistent kernel
log did, so I cannot show a clean before and after from there.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <marek@czernohous.de>
---
 .../gpu/drm/nouveau/nvkm/engine/fifo/nv04.c   | 26 ++++++++++++++-----
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/nv04.c b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/nv04.c
index c4b8e567d86f..ab144c1bd9da 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/nv04.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/nv04.c
@@ -327,12 +327,26 @@ nv04_fifo_intr_cache_error(struct nvkm_fifo *fifo, u32 chid, u32 get)
 
 	if (!(pull0 & 0x00000100) ||
 	    !nv04_fifo_swmthd(device, chid, mthd, data)) {
-		chan = nvkm_chan_get_chid(&fifo->engine, chid, &flags);
-		nvkm_error(subdev, "CACHE_ERROR - "
-			   "ch %d [%s] subc %d mthd %04x data %08x\n",
-			   chid, chan ? chan->name : "unknown",
-			   (mthd >> 13) & 7, mthd & 0x1ffc, data);
-		nvkm_chan_put(&chan, flags);
+		/*
+		 * Filter the benign Mesa bind probe: mthd 0x0060 with data
+		 * 0xbeef02xx is a harmless userspace probe and does not
+		 * indicate an actual error condition. The test is on the
+		 * method and data pattern alone, so it applies on every
+		 * chip that reaches this handler, not just on Tesla.
+		 * Demote to debug to keep dmesg clean while still catching
+		 * real CACHE_ERROR events.
+		 */
+		if ((mthd & 0x1ffc) == 0x0060 &&
+		    (data & 0xffffff00) == 0xbeef0200) {
+			nvkm_debug(subdev, "CACHE_ERROR - ch %d subc %d mthd %04x data %08x (benign, skipped)\n",
+				   chid, (mthd >> 13) & 7, mthd & 0x1ffc, data);
+		} else {
+			chan = nvkm_chan_get_chid(&fifo->engine, chid, &flags);
+			nvkm_error(subdev, "CACHE_ERROR - ch %d [%s] subc %d mthd %04x data %08x\n",
+				   chid, chan ? chan->name : "unknown",
+				   (mthd >> 13) & 7, mthd & 0x1ffc, data);
+			nvkm_chan_put(&chan, flags);
+		}
 	}
 
 	nvkm_wr32(device, NV04_PFIFO_CACHE1_DMA_PUSH, 0);
-- 
2.54.0

[PATCH v3 4/4] drm/nouveau: subscribe to channel-kill events on NV50 and newer

[Marek Czernohous]

From: Marek Czernohous <marek@czernohous.de>

nouveau_channel_init() only subscribes to the channel-killed event for
FERMI_CHANNEL_GPFIFO and newer. On NV50/Tesla the subscription therefore
never happens, and nvkm_chan_error()'s NVKM_CHAN_EVENT_ERRORED is
delivered into an empty notifier list.

Today that is harmless, because nothing kills a channel on Tesla: the
only nvkm_chan_error() callers are the Fermi and newer recovery paths.
So this patch changes no observable behaviour on its own, and that is
deliberate: it removes a latent trap before anything can fall into it.
I am carrying a Tesla recovery path that does add such a caller and will
send it separately once it is ready. Without a subscriber in place the
consequences there are severe: nouveau_channel_killed() never runs, so
nouveau_fence_context_kill() never runs either, and the pending fences
of the killed channel are never signalled. Everything waiting on them
waits forever: drm_atomic_helper_wait_for_fences() in the display commit
tail waits uninterruptibly and without a timeout, and the TTM delayed
delete workers wait in TASK_UNINTERRUPTIBLE. The user sees a frozen
desktop on a machine that is otherwise alive, and nothing in the kernel
ends that state: both waits pass MAX_SCHEDULE_TIMEOUT, so the fences
cannot time out. They are signalled only when the fence context is torn
down, that is when the DRM client owning the channel closes its fd and
nouveau_fence_context_del() runs. Killing the client, or rebooting,
clears it; waiting does not.
That is also a dma-fence contract violation: a fence must always be
signalled, with an error if necessary.

Lower the class gate to NV50_CHANNEL_GPFIFO. The nvkm side is already
class neutral: the KILLED case hangs the notifier on runl->chid->event,
which every fifo owns since the runlist rework, and nvkm_uchan_uevent()
does not discriminate by class. Pre-NV50 chips keep the old behaviour,
so NV04 to NV40 are unaffected.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <marek@czernohous.de>
---
 drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/nouveau/nouveau_chan.c
index 07b0bd1bc519..5c2f4b9342b7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -485,7 +485,7 @@ nouveau_channel_init(struct nouveau_channel *chan, u32 vram, u32 gart)
 	 * lock and the pending list, so an event arriving in between would
 	 * find a non-NULL but unusable context and walk a NULL list head.
 	 */
-	if (chan->user.oclass >= FERMI_CHANNEL_GPFIFO) {
+	if (chan->user.oclass >= NV50_CHANNEL_GPFIFO) {
 		DEFINE_RAW_FLEX(struct nvif_event_v0, args, data,
 				sizeof(struct nvif_chan_event_v0));
 		struct nvif_chan_event_v0 *host =
-- 
2.54.0

[PATCH v3 4/4] drm/nouveau: subscribe to channel-kill events on NV50 and newer

[Marek Czernohous]

From: Marek Czernohous <marek@czernohous.de>

nouveau_channel_init() only subscribes to the channel-killed event for
FERMI_CHANNEL_GPFIFO and newer. On NV50/Tesla the subscription therefore
never happens, and nvkm_chan_error()'s NVKM_CHAN_EVENT_ERRORED is
delivered into an empty notifier list.

Today that is harmless, because nothing kills a channel on Tesla: the
only nvkm_chan_error() callers are the Fermi and newer recovery paths.
So this patch changes no observable behaviour on its own, and that is
deliberate: it removes a latent trap before anything can fall into it.
I am carrying a Tesla recovery path that does add such a caller and will
send it separately once it is ready. Without a subscriber in place the
consequences there are severe: nouveau_channel_killed() never runs, so
nouveau_fence_context_kill() never runs either, and the pending fences
of the killed channel are never signalled. Everything waiting on them
waits forever: drm_atomic_helper_wait_for_fences() in the display commit
tail waits uninterruptibly and without a timeout, and the TTM delayed
delete workers wait in TASK_UNINTERRUPTIBLE. The user sees a frozen
desktop on a machine that is otherwise alive, and nothing in the kernel
ends that state: both waits pass MAX_SCHEDULE_TIMEOUT, so the fences
cannot time out. They are signalled only when the fence context is torn
down, that is when the DRM client owning the channel closes its fd and
nouveau_fence_context_del() runs. Killing the client, or rebooting,
clears it; waiting does not.
That is also a dma-fence contract violation: a fence must always be
signalled, with an error if necessary.

Lower the class gate to NV50_CHANNEL_GPFIFO. The nvkm side is already
class neutral: the KILLED case hangs the notifier on runl->chid->event,
which every fifo owns since the runlist rework, and nvkm_uchan_uevent()
does not discriminate by class. Pre-NV50 chips keep the old behaviour,
so NV04 to NV40 are unaffected.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <marek@czernohous.de>
---
 drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/nouveau/nouveau_chan.c
index 07b0bd1bc519..5c2f4b9342b7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -485,7 +485,7 @@ nouveau_channel_init(struct nouveau_channel *chan, u32 vram, u32 gart)
 	 * lock and the pending list, so an event arriving in between would
 	 * find a non-NULL but unusable context and walk a NULL list head.
 	 */
-	if (chan->user.oclass >= FERMI_CHANNEL_GPFIFO) {
+	if (chan->user.oclass >= NV50_CHANNEL_GPFIFO) {
 		DEFINE_RAW_FLEX(struct nvif_event_v0, args, data,
 				sizeof(struct nvif_chan_event_v0));
 		struct nvif_chan_event_v0 *host =
-- 
2.54.0

Re: [PATCH v3 2/4] drm/nouveau: subscribe to the channel-kill event after the fence context

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Teardown symmetry bug in `nouveau_fence_context_del` can lead to UAF of `fctx` via a rescheduled work item.
--

--- Patch [2]: [PATCH v3 2/4] drm/nouveau: subscribe to the channel-kill event after the fence context ---
commit 0d333033075f8f241623e82cf6a4863bcbfcda8f
Author: Marek Czernohous <marek@czernohous.de>

drm/nouveau: subscribe to the channel-kill event after the fence context

nouveau_channel_init() arms the channel-kill subscription early, right
after mapping userd, and only creates the fence context at the very end
of the same function. The handler it installs, nouveau_channel_killed(),
reaches nouveau_fence_context_kill(chan->fence).

> diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/nouveau/nouveau_chan.c
> index f142f63105966..07b0bd1bc519f 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_chan.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
[ ... ]

While reviewing this, I noticed a pre-existing issue in the fence teardown
sequence:

drivers/gpu/drm/nouveau/nouveau_fence.c:nouveau_fence_context_del() {
	cancel_work_sync(&fctx->uevent_work);
	nouveau_fence_context_kill(fctx, 0);
	nvif_event_dtor(&fctx->event);
	fctx->dead = 1;
...
}

[Severity: High]
This is a pre-existing issue, but does this teardown order create a
use-after-free risk for fctx?

If userspace emits a fence with signaling enabled and then immediately closes
the channel, nouveau_fence_context_del() is called. It executes
cancel_work_sync(&fctx->uevent_work) before stopping the event source.

If a hardware interrupt for the fence fires immediately after
cancel_work_sync() returns but before nouveau_fence_context_kill() blocks the
event, the interrupt handler nouveau_fence_wait_uevent_handler() will execute
and call schedule_work(&fctx->uevent_work).

The teardown sequence then continues, freeing the fctx memory via
nouveau_fence_context_free(). When the asynchronously scheduled work executes
later, nouveau_fence_uevent_work() accesses the freed fctx, resulting in a
use-after-free.

Should the event source be blocked or destroyed before synchronizing the work
cancellation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812231330.705425-1-mczernohous@gmail.com?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Canceling `uevent_work` before destroying `fctx->event` in `nouveau_fence_context_del` leaves a window for use-after-free.
- [High] `nouveau_connector_destroy()` frees the connector without cancelling `nv_connector->irq_work`, leading to a UAF.
--

--- Patch [1]: [PATCH v3 1/4] drm/nouveau: unsubscribe the channel-kill event before the fence context ---
commit a14b59dbc876448fa7fc08aa6ef35ba9137e4909
Author: Marek Czernohous <marek@czernohous.de>

drm/nouveau: unsubscribe the channel-kill event before the fence context

nouveau_channel_del() tears the fence context down first and only drops
the channel-kill subscription later. This leaves a race window where a kill
event can be delivered and attempt to access a freed fence context. This
patch drops the subscription before anything it depends on is torn down.

Fixes: ea13e5abf807 ("drm/nouveau: signal pending fences when channel has been killed")
Cc: stable@vger.kernel.org

> diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/nouveau/nouveau_chan.c
> index 598513f604493..f142f63105966 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);

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a similar pattern
in nouveau_fence_context_del() inside 
drivers/gpu/drm/nouveau/nouveau_fence.c:

void
nouveau_fence_context_del(struct nouveau_fence_chan *fctx)
{
	cancel_work_sync(&fctx->uevent_work);
	nouveau_fence_context_kill(fctx, 0);
	nvif_event_dtor(&fctx->event);
        ...

If a hardware event triggers exactly in the window between cancel_work_sync()
and nvif_event_dtor(), nouveau_fence_wait_uevent_handler() unconditionally
reschedules the work via schedule_work(&fctx->uevent_work). 

Does this leave a window where the work remains on the queue after the fctx
structure is freed, resulting in a use-after-free?


[Severity: High]
This is also a pre-existing issue, but there is a similar teardown problem
in nouveau_connector_destroy() inside 
drivers/gpu/drm/nouveau/nouveau_connector.c:

static void
nouveau_connector_destroy(struct drm_connector *connector)
{
	struct nouveau_connector *nv_connector = nouveau_connector(connector);
	nvif_event_dtor(&nv_connector->irq);
	nvif_event_dtor(&nv_connector->hpd);
        ...
	kfree(connector);
}

The connector is freed without calling cancel_work_sync(&nv_connector->irq_work)
to cancel any previously scheduled work. 

If an interrupt fires just before or during nvif_event_dtor(), the work is
successfully scheduled on the system queue. When the queued work executes, it
will dereference the freed nv_connector pointer. 

Should this function cancel the irq_work before freeing the connector?

[ ... ]

> @@ -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);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812231330.705425-1-mczernohous@gmail.com?part=1