Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Asymmetric teardown of `chan->kill` event and `chan->fence` leads to a Use-After-Free if a channel kill event fires during channel destruction. - [High] Channel kill event can fire before `chan->fence` is fully initialized, leading to uninitialized spinlock and list head access. -- --- Patch [2]: [PATCH v2 2/3] drm/nouveau: subscribe to channel-kill events on NV50 and newer --- commit 66f8740c572f7ea5ed5181be09ddea5061fdb648 Author: Marek Czernohous <[email protected]> drm/nouveau: subscribe to channel-kill events on NV50 and newer nouveau_channel_init() only subscribes to the channel-killed event for FERMI_CHANNEL_GPFIFO and newer. This patch lowers the class gate to NV50_CHANNEL_GPFIFO, ensuring that pending fences on a killed channel are properly signalled for NV50/Tesla hardware. > diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c > b/drivers/gpu/drm/nouveau/nouveau_chan.c > index 598513f604493..1db9fbd81f5c0 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_chan.c > +++ b/drivers/gpu/drm/nouveau/nouveau_chan.c > @@ -363,7 +363,7 @@ nouveau_channel_init(struct nouveau_channel *chan, u32 > vram, u32 gart) > if (ret) > return ret; > > - if (chan->user.oclass >= FERMI_CHANNEL_GPFIFO) { > + if (chan->user.oclass >= NV50_CHANNEL_GPFIFO) { [Severity: High] This is a pre-existing issue, but does expanding this subscription to NV50 expose it to a use-after-free during channel teardown? During teardown, nouveau_channel_del() destroys the fence context before unregistering the channel kill event: nouveau_channel_del() { ... if (chan->fence) nouveau_fence(chan->cli->drm)->context_del(chan); ... nvif_event_dtor(&chan->kill); } The fence context is freed in nv10_fence_context_del() where chan->fence is set to NULL non-atomically. If a channel kill event fires concurrently, could the asynchronous handler nouveau_channel_kill() read chan->fence locklessly just before it is set to NULL and dereference the freed fence context inside nouveau_fence_context_kill()? [Severity: High] This isn't a bug introduced by this patch, but is there also a race during initialization where the kill event can fire before the fence context is fully initialized? In nouveau_channel_init(), the kill event is enabled early: nouveau_channel_init() { ... if (ret == 0) ret = nvif_event_allow(&chan->kill); ... } However, the fence context is created at the very end of the function: nouveau_channel_init() { ... return nouveau_fence(drm)->context_new(chan); } Inside nv50_fence_context_new(), chan->fence is assigned via kzalloc before the underlying lists and spinlocks are initialized by nouveau_fence_context_new(): nv50_fence_context_new() { ... fctx = chan->fence = kzalloc_obj(*fctx); if (!fctx) return -ENOMEM; nouveau_fence_context_new(chan, &fctx->base); } If a kill event fires during this window, could nouveau_fence_context_kill() attempt to acquire an uninitialized spinlock and traverse an uninitialized list? > DEFINE_RAW_FLEX(struct nvif_event_v0, args, data, > sizeof(struct nvif_chan_event_v0)); > struct nvif_chan_event_v0 *host = -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
