[61/75] drm/sun4i: tv: Convert to atomic_create_state
Commit Message
The sun4i TV connector reset implementation creates a pristine state
and initializes the analog TV properties. It does not perform any
hardware reset.
Since this is equivalent to what atomic_create_state expects, convert
the reset function to an atomic_create_state hook that returns the
initialized state.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: Chen-Yu Tsai <wens@kernel.org>
Cc: Samuel Holland <samuel@sholland.org>
Cc: linux-sunxi@lists.linux.dev
---
drivers/gpu/drm/sun4i/sun4i_tv.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
Comments
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 <mripard@kernel.org>
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;
> }
@@ -403,20 +403,28 @@ static const struct drm_encoder_helper_funcs sun4i_tv_helper_funcs = {
static const struct drm_connector_helper_funcs sun4i_tv_comp_connector_helper_funcs = {
.atomic_check = drm_atomic_helper_connector_tv_check,
.get_modes = drm_connector_helper_tv_get_modes,
};
-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);
+
+ return connector_state;
}
static const struct drm_connector_funcs sun4i_tv_comp_connector_funcs = {
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = drm_connector_cleanup,
- .reset = sun4i_tv_connector_reset,
+ .atomic_create_state = sun4i_tv_connector_create_state,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
static const struct regmap_config sun4i_tv_regmap_config = {