Re: [PATCH v8 14/14] drm/i915/display: use drm backlight

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unprotected access to `crtc->state` in `drm_atomic_helper_connector_apply_luminance` leading to Use-After-Free.
--

commit 4d692fcf914aa54e6baac83f8855bd80382896c8
Author: Mario Limonciello <mario.limonciello@amd.com>
Subject: drm/i915/display: use drm backlight

Convert the Intel display drivers to the DRM backlight infrastructure so
brightness can be controlled through the connector LUMINANCE property. The
DRM core owns the embedded backlight state and its property.

> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 9dfd095f325f7..d53e5f4bdf57d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
[ ... ]
> @@ -7608,6 +7610,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>  	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
>  	display->modeset.funcs->commit_modeset_enables(state);
>  
> +	for_each_new_connector_in_state(&state->base, connector, new_conn_state, i)
> +		drm_atomic_helper_connector_apply_luminance(new_conn_state);
> +

[Severity: High]
Could this lead to a Use-After-Free of the CRTC state?

When userspace updates only the LUMINANCE property, the CRTC is not pulled
into the atomic state, meaning crtc->mutex is never locked and no
serialization dependencies are established.

In intel_atomic_commit_tail(), we call
drm_atomic_helper_connector_apply_luminance() which dereferences the
global CRTC state:

drivers/gpu/drm/drm_atomic_helper.c:drm_atomic_helper_connector_apply_luminance() {
	struct drm_connector *connector = conn_state->connector;

	if (conn_state->crtc && conn_state->crtc->state &&
	    conn_state->crtc->state->active)
		drm_backlight_set_luminance(connector, conn_state->luminance);
}

Since the LUMINANCE commit executes concurrently with no CRTC
synchronization, another thread performing a modeset or DPMS update could
simultaneously acquire crtc->mutex, call drm_atomic_helper_swap_state(),
and free the old CRTC state while this code evaluates ->active.

Is this safe without acquiring the CRTC lock or pulling the CRTC into the
atomic transaction?

>  	intel_display_power_dc3co_compute(state);
>  
>  	/* FIXME probably need to sequence this properly */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908044035.62093-1-mario.limonciello@amd.com?part=14