Re: [PATCH v2 01/13] drm/crtc: Introduce hw_reset helper hook

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing NULL pointer check for optional `crtc_funcs` before checking for `hw_reset`
--

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

drm/crtc: Introduce hw_reset helper hook

This commit introduces a new hw_reset hook in struct drm_crtc_helper_funcs to
allow resetting hardware to a known good state without affecting software state.
It updates drm_mode_config_crtc_reset_with_create_state() to call this hook
after the software state is successfully created.

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

[ ... ]

> @@ -230,12 +231,22 @@ static int drm_mode_config_crtc_create_state(struct drm_crtc *crtc)
>  
>  static int drm_mode_config_crtc_reset_with_create_state(struct drm_crtc *crtc)
>  {
> +	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> +	int ret;
> +
>  	if (crtc->state) {
>  		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
>  		crtc->state = NULL;
>  	}
>  
> -	return drm_mode_config_crtc_create_state(crtc);
> +	ret = drm_mode_config_crtc_create_state(crtc);
> +	if (ret)
> +		return ret;
> +
> +	if (crtc_funcs->hw_reset)
> +		crtc_funcs->hw_reset(crtc);

[Severity: High]
Could this lead to a NULL pointer dereference for drivers that do not use the
CRTC helpers?

If a driver implements atomic_create_state but doesn't use the CRTC helpers,
crtc->helper_private will be NULL. When drm_mode_config_reset() iterates
over all CRTCs and calls drm_mode_config_crtc_reset_with_create_state(),
crtc_funcs will evaluate to NULL.

Since crtc_funcs is unconditionally dereferenced here to check for hw_reset,
would this cause a panic during initialization or suspend/resume paths?

Should there be an additional check to ensure crtc_funcs is valid?

	if (crtc_funcs && crtc_funcs->hw_reset)
		crtc_funcs->hw_reset(crtc);

> +
> +	return 0;
>  }
>  
>  static int drm_mode_config_connector_create_state(struct drm_connector *connector)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-drm-no-more-crtc-reset-v2-0-4500d49a337f@kernel.org?part=1