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 <[email protected]>

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/[email protected]?part=1

Reply via email to