Re: [PATCH 2/3] drm/nouveau: cancel the DP IRQ work before freeing the connector

[lyude@redhat.com]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Sat, 2026-08-15 at 21:54 +0200, Marek Czernohous wrote:
> From: Marek Czernohous <marek@czernohous.de>
> 
> nouveau_connector_destroy() tears the two nvif events down and then
> frees the connector, but never cancels the work the IRQ event queues:
> 
> 	nvif_event_dtor(&nv_connector->irq);
> 	nvif_event_dtor(&nv_connector->hpd);
> 	kfree(nv_connector->edid);
> 	...
> 	kfree(connector);
> 
> nouveau_connector_irq() queues that work unconditionally:
> 
> 	schedule_work(&nv_connector->irq_work);
> 	return NVIF_EVENT_KEEP;
> 
> A DP IRQ arriving just before nvif_event_dtor() therefore leaves
> nv_connector->irq_work on the system queue past the kfree(). When it
> runs, nouveau_dp_irq() derives both nv_connector and connector from
> the
> work_struct and dereferences them, and goes on to take
> outp->dp.hpd_irq_lock.
> 
> There is no cancel_work_sync() for irq_work anywhere in the driver,
> so
> nothing else covers this. Add it after the event teardown, where no
> further work can be queued, and before anything is freed.
> 
> Reported by the Sashiko review bot as a pre-existing issue, in its
> review
> of an earlier nv04 FIFO series of mine, and confirmed against the
> source.
> 
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Link:
> https://sashiko.dev/#/patchset/20260812231330.705425-1-mczernohous@gmail.com?part=1
> Fixes: 773eb04d14a1 ("drm/nouveau/disp: expose conn event class")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Marek Czernohous <marek@czernohous.de>
> ---
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index b0b0ad9a0c24..e49dcaa6d210 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -397,6 +397,7 @@ 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);
> +	cancel_work_sync(&nv_connector->irq_work);
>  	kfree(nv_connector->edid);
>  	drm_connector_unregister(connector);
>  	drm_connector_cleanup(connector);