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

Reviewed-by: Lyude Paul <[email protected]>

On Sat, 2026-08-15 at 21:54 +0200, Marek Czernohous wrote:
> From: Marek Czernohous <[email protected]>
> 
> 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 <[email protected]>
> Link:
> https://sashiko.dev/#/patchset/[email protected]?part=1
> Fixes: 773eb04d14a1 ("drm/nouveau/disp: expose conn event class")
> Cc: [email protected]
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Marek Czernohous <[email protected]>
> ---
>  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);