Re: [PATCH 64/75] drm/vc4: vec: Convert to atomic_create_state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/vc4: NULL pointer deref in drm_atomic_helper_connector_tv_state_init
--

commit 0f9d479612175281e5b2697d74e34321440ab363
Author: Maxime Ripard <mripard@kernel.org>

drm/vc4: vec: Convert to atomic_create_state

This commit converts the vc4 VEC connector reset implementation to an
atomic_create_state hook. It creates a pristine state and initializes
the analog TV properties without performing hardware resets.

> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> --- a/drivers/gpu/drm/vc4/vc4_vec.c
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c

[ ... ]

> -static void vc4_vec_connector_reset(struct drm_connector *connector)
> +static struct drm_connector_state *
> +vc4_vec_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]
Will this cause a NULL pointer dereference during display initialization?

The core DRM function drm_mode_config_connector_reset_with_create_state()
explicitly sets connector->state to NULL before invoking the
atomic_create_state hook.

This means connector->state is NULL when
drm_atomic_helper_connector_tv_state_init() is called here.

Looking at drm_atomic_helper_connector_tv_state_init() in
drm_atomic_state_helper.c, it ignores the local state parameter when
initializing margins and instead passes connector->state:

void drm_atomic_helper_connector_tv_state_init(
        struct drm_connector_state *state,
        struct drm_connector *connector)
{
        ...
        drm_atomic_helper_connector_tv_margins_state_init(connector->state,
                                                          connector);
}

Because connector->state is NULL in this context, does calling this
helper result in a guaranteed crash when the margins initializer
attempts to dereference the state parameter?

> +
> +	return connector_state;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-drm-no-more-connector-reset-v1-0-586c9c026823@kernel.org?part=64