[PATCH] drm/amd/display: Fix error pointers in amdgpu_dm_crtc_mem_type_changed

[Li, Roman]

[Public]

Reviewed-by: Roman Li <roman.li at amd.com>

> -----Original Message-----
> From: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>
> Sent: Wednesday, January 15, 2025 12:07 PM
> To: Siqueira, Rodrigo <Rodrigo.Siqueira at amd.com>; Pillai, Aurabindo
> <Aurabindo.Pillai at amd.com>
> Cc: amd-gfx at lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> <SRINIVASAN.SHANMUGAM at amd.com>; Li, Sun peng (Leo)
> <Sunpeng.Li at amd.com>; Chung, ChiaHsuan (Tom)
> <ChiaHsuan.Chung at amd.com>; Li, Roman <Roman.Li at amd.com>; Hung, Alex
> <Alex.Hung at amd.com>; Wentland, Harry <Harry.Wentland at amd.com>; Hamza
> Mahfooz <hamza.mahfooz at amd.com>; Dan Carpenter <dan.carpenter at linaro.org>
> Subject: [PATCH] drm/amd/display: Fix error pointers in
> amdgpu_dm_crtc_mem_type_changed
>
> The function amdgpu_dm_crtc_mem_type_changed was dereferencing pointers
> returned by drm_atomic_get_plane_state without checking for errors. This could lead
> to undefined behavior if the function returns an error pointer.
>
> This commit adds checks using IS_ERR to ensure that new_plane_state and
> old_plane_state are valid before dereferencing them.
>
> Fixes the below:
>
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:11486
> amdgpu_dm_crtc_mem_type_changed()
> error: 'new_plane_state' dereferencing possible ERR_PTR()
>
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
>     11475 static bool amdgpu_dm_crtc_mem_type_changed(struct drm_device *dev,
>     11476                                             struct drm_atomic_state *state,
>     11477                                             struct drm_crtc_state *crtc_state)
>     11478 {
>     11479         struct drm_plane *plane;
>     11480         struct drm_plane_state *new_plane_state, *old_plane_state;
>     11481
>     11482         drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
>     11483                 new_plane_state = drm_atomic_get_plane_state(state, plane);
>     11484                 old_plane_state = drm_atomic_get_plane_state(state, plane);
>                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^ These functions can fail.
>
>     11485
> --> 11486                 if (old_plane_state->fb && new_plane_state->fb &&
>     11487                     get_mem_type(old_plane_state->fb) !=
> get_mem_type(new_plane_state->fb))
>     11488                         return true;
>     11489         }
>     11490
>     11491         return false;
>     11492 }
>
> Fixes: 1079bd90c55b ("drm/amd/display: Do not elevate mem_type change to full
> update")
> Cc: Leo Li <sunpeng.li at amd.com>
> Cc: Tom Chung <chiahsuan.chung at amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> Cc: Roman Li <roman.li at amd.com>
> Cc: Alex Hung <alex.hung at amd.com>
> Cc: Aurabindo Pillai <aurabindo.pillai at amd.com>
> Cc: Harry Wentland <harry.wentland at amd.com>
> Cc: Hamza Mahfooz <hamza.mahfooz at amd.com>
> Reported-by: Dan Carpenter <dan.carpenter at linaro.org>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index fe75fbced027..f3f319238763 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -11523,6 +11523,11 @@ static bool
> amdgpu_dm_crtc_mem_type_changed(struct drm_device *dev,
>               new_plane_state = drm_atomic_get_plane_state(state, plane);
>               old_plane_state = drm_atomic_get_plane_state(state, plane);
>
> +             if (IS_ERR(new_plane_state) || IS_ERR(old_plane_state)) {
> +                     DRM_ERROR("Failed to get plane state for plane %s\n",
> plane->name);
> +                     return false;
> +             }
> +
>               if (old_plane_state->fb && new_plane_state->fb &&
>                   get_mem_type(old_plane_state->fb) !=
> get_mem_type(new_plane_state->fb))
>                       return true;
> --
> 2.34.1