[Public]
Reviewed-by: Roman Li <roman...@amd.com>
> -----Original Message-----
> From: SHANMUGAM, SRINIVASAN <srinivasan.shanmu...@amd.com>
> Sent: Wednesday, January 15, 2025 12:07 PM
> To: Siqueira, Rodrigo <rodrigo.sique...@amd.com>; Pillai, Aurabindo
> <aurabindo.pil...@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> <srinivasan.shanmu...@amd.com>; Li, Sun peng (Leo)
> <sunpeng...@amd.com>; Chung, ChiaHsuan (Tom)
> <chiahsuan.ch...@amd.com>; Li, Roman <roman...@amd.com>; Hung, Alex
> <alex.h...@amd.com>; Wentland, Harry <harry.wentl...@amd.com>; Hamza
> Mahfooz <hamza.mahf...@amd.com>; Dan Carpenter <dan.carpen...@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...@amd.com>
> Cc: Tom Chung <chiahsuan.ch...@amd.com>
> Cc: Rodrigo Siqueira <rodrigo.sique...@amd.com>
> Cc: Roman Li <roman...@amd.com>
> Cc: Alex Hung <alex.h...@amd.com>
> Cc: Aurabindo Pillai <aurabindo.pil...@amd.com>
> Cc: Harry Wentland <harry.wentl...@amd.com>
> Cc: Hamza Mahfooz <hamza.mahf...@amd.com>
> Reported-by: Dan Carpenter <dan.carpen...@linaro.org>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmu...@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
