Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Discarded return value of atomic_create_state() leads to leaked memory 
and NULL pointer dereference.
--

--- Patch [60]: [PATCH 60/60] drm/plane: Remove reset ---
commit 272441f81f14a41d0a23a7e99bf3963a3955d652
Author: Maxime Ripard <[email protected]>

    drm/plane: Remove reset
    
    All plane drivers now implement the atomic_create_state hook instead of
    the reset hook for initial state creation. The reset hook conflated
    initial state allocation at probe time with hardware and software reset
    during suspend/resume, making error handling difficult since it is not
    fallible.
    
    Remove the reset hook from struct drm_plane_funcs and the associated
    call in drm_mode_config_reset().

> diff --git a/drivers/gpu/drm/drm_mode_config.c 
> b/drivers/gpu/drm/drm_mode_config.c
> index f432f485a914c..4b07189e3be5e 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -289,9 +289,7 @@ void drm_mode_config_reset(struct drm_device *dev)
>               drm_colorop_reset(colorop);
>  
>       drm_for_each_plane(plane, dev) {
> -             if (plane->funcs->reset)
> -                     plane->funcs->reset(plane);
> -             else if (plane->funcs->atomic_create_state)
> +             if (plane->funcs->atomic_create_state)
>                       drm_mode_config_plane_reset_with_create_state(plane);
>       }

[Severity: High]
With the transition to atomic_create_state, is there a missing assignment for
AMDGPU displays during initialization?

In drm_mode_config_reset() above, the core handles the new hook by calling
drm_mode_config_plane_reset_with_create_state(), which properly assigns the
allocated state to plane->state.

However, AMDGPU does not call drm_mode_config_reset(). In
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c,
amdgpu_dm_plane_init() calls the new hook manually but discards the result:

        /* Create (reset) the plane state */
        if (plane->funcs->atomic_create_state)
                plane->funcs->atomic_create_state(plane);

Unlike the old reset hook which assigned plane->state internally (via
__drm_atomic_helper_plane_reset()), atomic_create_state() just returns
the new state pointer. Discarding this return value leaves plane->state as
NULL and leaks the allocated state.

Will this cause a NULL pointer dereference on AMD platforms later in the boot
process, such as during drm_client_modeset_probe(), when
atomic_duplicate_state() attempts to access plane->state?

[ ... ]

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

Reply via email to