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

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 <[email protected]>

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/[email protected]?part=2