Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm: Incomplete state decoupling causes NULL deref in tv_margins_state_init --
commit 44e808ff1921e4945725ba2420217e2015fc2db5 Author: Maxime Ripard <[email protected]> drm/atomic-state-helper: Rename connector_tv_reset to state_init This commit renames drm_atomic_helper_connector_tv_reset to drm_atomic_helper_connector_tv_state_init. It adds a connector state parameter to decouple the initialization from the reset hook, preparing it for contexts like atomic_create_state where the connector state isn't assigned yet. > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c > b/drivers/gpu/drm/drm_atomic_state_helper.c > index 6238066fd5d60..1424c4873251c 100644 > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c [ ... ] > @@ -501,16 +501,17 @@ void > drm_atomic_helper_connector_tv_margins_state_init(struct drm_connector_stat > EXPORT_SYMBOL(drm_atomic_helper_connector_tv_margins_state_init); > > /** > - * drm_atomic_helper_connector_tv_reset - Resets Analog TV connector > properties > + * drm_atomic_helper_connector_tv_state_init - Initializes Analog TV > connector properties > + * @state: the DRM State object > * @connector: DRM connector > * > - * Resets the analog TV properties attached to a connector > + * Initializes the analog TV properties attached to a connector > */ > -void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector) > +void drm_atomic_helper_connector_tv_state_init(struct drm_connector_state > *state, > + struct drm_connector *connector) > { > struct drm_device *dev = connector->dev; > struct drm_cmdline_mode *cmdline = &connector->cmdline_mode; > - struct drm_connector_state *state = connector->state; > struct drm_property *prop; > uint64_t val; > [ ... ] > @@ -574,7 +575,7 @@ void drm_atomic_helper_connector_tv_reset(struct > drm_connector *connector) > drm_atomic_helper_connector_tv_margins_state_init(connector->state, > connector); [Severity: High] Since the new state parameter is added to decouple this function from the reset hook, should this call pass the newly provided state instead of connector->state? When a driver like vc4_vec calls this via its atomic_create_state hook, connector->state is not yet assigned and is NULL. Passing connector->state here forwards a NULL pointer to drm_atomic_helper_connector_tv_margins_state_init(), which then writes directly to state->tv.margins.left, leading to a NULL pointer dereference during device probe: void drm_atomic_helper_connector_tv_margins_state_init(struct drm_connector_state *state, struct drm_connector *connector) { struct drm_cmdline_mode *cmdline = &connector->cmdline_mode; state->tv.margins.left = cmdline->tv_margins.left; > } > -EXPORT_SYMBOL(drm_atomic_helper_connector_tv_reset); > +EXPORT_SYMBOL(drm_atomic_helper_connector_tv_state_init); -- Sashiko AI review · https://sashiko.dev/#/patchset/20261014-drm-no-more-connector-reset-v1-0-586c9c026...@kernel.org?part=56
