Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm: NULL pointer dereference when initializing TV connector state --
commit a85a6a0ad13c063d072cdcda87267ea8867cafa3 Author: Maxime Ripard <[email protected]> drm/sun4i: tv: Convert to atomic_create_state The sun4i TV connector reset implementation is converted to an atomic_create_state hook. It creates the pristine state and initializes the analog TV properties. > diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c > b/drivers/gpu/drm/sun4i/sun4i_tv.c > index ea86f8a6a35e7..a68e5f7a03d2c 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tv.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c [ ... ] > -static void sun4i_tv_connector_reset(struct drm_connector *connector) > +static struct drm_connector_state * > +sun4i_tv_connector_create_state(struct drm_connector *connector) > { > - drm_atomic_helper_connector_reset(connector); > - drm_atomic_helper_connector_tv_state_init(connector->state, connector); > + struct drm_connector_state *connector_state; > + > + connector_state = drm_atomic_helper_connector_create_state(connector); > + if (IS_ERR(connector_state)) > + return connector_state; > + > + drm_atomic_helper_connector_tv_state_init(connector_state, connector); [Severity: High] Does this trigger a NULL pointer dereference during initialization? When the DRM core resets the connector state, it explicitly clears the old state before invoking the .atomic_create_state hook: drm_mode_config_connector_reset_with_create_state() { ... if (connector->state) { ... connector->state = NULL; } return drm_mode_config_connector_create_state(connector); } The core helper drm_atomic_helper_connector_tv_state_init() then mistakenly passes the unassigned connector->state down to the margins initialization instead of the newly allocated state: drm_atomic_helper_connector_tv_state_init() { ... drm_atomic_helper_connector_tv_margins_state_init(connector->state, connector); } This causes drm_atomic_helper_connector_tv_margins_state_init() to unconditionally dereference the NULL state pointer: drm_atomic_helper_connector_tv_margins_state_init(state, ...) { ... state->tv.margins.left = cmdline->tv_margins.left; ... } The previous .reset hook implementation masked this core helper bug because it assigned connector->state prior to calling the TV state initialization helper. Could we fix the core helper to pass the correct state pointer, or should this driver handle the initialization differently to avoid the crash? > + > + return connector_state; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/20261014-drm-no-more-connector-reset-v1-0-586c9c026...@kernel.org?part=61
