Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Unchecked dereference of optional `crtc->helper_private` in 
`drm_mode_config_crtc_reset_with_create_state()` before calling `hw_reset`.
--

commit c111ccec175b0bcef9dba73f9b4a2e31404bd779
Author: Maxime Ripard <[email protected]>
drm/crtc: Remove reset

This commit removes the reset hook from struct drm_crtc_funcs. As a result,
all crtc drivers now rely on the atomic_create_state hook for initial state
creation, and drm_mode_config_reset() is updated to reflect this.

> diff --git a/drivers/gpu/drm/drm_mode_config.c 
> b/drivers/gpu/drm/drm_mode_config.c
> index bb2efc274323e..5eed5c194b661 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -307,9 +307,7 @@ void drm_mode_config_reset(struct drm_device *dev)
>       }
>  
>       drm_for_each_crtc(crtc, dev) {
> -             if (crtc->funcs->reset)
> -                     crtc->funcs->reset(crtc);
> -             else if (crtc->funcs->atomic_create_state)
> +             if (crtc->funcs->atomic_create_state)
>                       drm_mode_config_crtc_reset_with_create_state(crtc);

[Severity: High]
This is a pre-existing issue, but does removing the fallback to the reset
hook expose a kernel panic during initialization or resume?

By forcing all CRTCs that implement atomic_create_state through
drm_mode_config_crtc_reset_with_create_state(), we unconditionally hit a path
that dereferences the optional helper_private pointer without checking if it
exists.

Looking at drm_mode_config_crtc_reset_with_create_state() in
drivers/gpu/drm/drm_mode_config.c:

drm_mode_config_crtc_reset_with_create_state() {
        const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
        ...
        if (crtc_funcs->hw_reset)
                crtc_funcs->hw_reset(crtc);
        ...
}

If an atomic DRM driver provides its own atomic implementations and does not
use the CRTC helpers, helper_private will be NULL. In that case, crtc_funcs
is NULL and the check for crtc_funcs->hw_reset will cause a NULL pointer
dereference.

>       }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=13

Reply via email to