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 <[email protected]> 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/[email protected]?part=14
