Re: [PATCH RFC 04/12] drm/i915/display: Switch to drmm_mode_config_init() and drop manual cleanup

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Deferring `intel_mode_config_cleanup` to a `drmm` action causes Use-After-Free (UAF) crashes during driver teardown because dependent resources are prematurely destroyed.
--

--- Patch [4]: [PATCH RFC 04/12] drm/i915/display: Switch to drmm_mode_config_init() and drop manual cleanup ---
commit 6ace97a8635ec8a367a634fb62172f65c853696d
Author: Kory Maincent <[email protected]>

drm/i915/display: Switch to drmm_mode_config_init() and drop manual cleanup

Replace drm_mode_config_init() with drmm_mode_config_init() in
intel_mode_config_init(). Remove the manual intel_mode_config_cleanup()
calls during teardown since the managed variant registers the cleanup with
devres automatically. Register intel_atomic_global_obj_cleanup() as a devres
action to run before drm_mode_config_cleanup().

> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 23bfecc983e8d..d02393053cef4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c

[ ... ]

> @@ -618,8 +631,6 @@ void intel_display_driver_remove_noirq(struct intel_display *display)
>  
>  	intel_hdcp_component_fini(display);
>  
> -	intel_mode_config_cleanup(display);
> -
>  	intel_dp_tunnel_mgr_cleanup(display);
>  
>  	intel_overlay_cleanup(display);

[Severity: High]
Does deferring intel_mode_config_cleanup() here cause a use-after-free during
driver teardown?

Because intel_mode_config_cleanup() is removed here, mode config cleanup (which 
frees connectors and encoders) is deferred to the asynchronous drmm phase. 
However, intel_dp_tunnel_mgr_cleanup() and the destruction of the display
workqueues (which occur later in this function) still happen synchronously.

This inverts the teardown order so the tunnel manager and workqueues are freed
before the encoders that depend on them. When the deferred 
drm_mode_config_cleanup() finally runs, it destroys encoders which triggers
intel_dp_tunnel_destroy(). This dereferences the dangling dp->tunnel pointer and 
the already freed tunnel->group->mgr.

Additionally, because delayed works (like intel_dp->pps.panel_vdd_work) are 
only cancelled during the deferred encoder cleanup in intel_pps_vdd_off_sync(), 
their timers may fire after destroy_workqueue() has already freed the workqueue, 
causing queue_work() to panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4