[Public]
Reviewed-by: Roman Li <roman.li@xxxxxxx>
> -----Original Message-----
> From: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@xxxxxxx>
> Sent: Wednesday, January 15, 2025 12:07 PM
> To: Siqueira, Rodrigo <Rodrigo.Siqueira@xxxxxxx>; Pillai, Aurabindo
> <Aurabindo.Pillai@xxxxxxx>
> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; SHANMUGAM, SRINIVASAN
> <SRINIVASAN.SHANMUGAM@xxxxxxx>; Li, Sun peng (Leo)
> <Sunpeng.Li@xxxxxxx>; Chung, ChiaHsuan (Tom)
> <ChiaHsuan.Chung@xxxxxxx>; Li, Roman <Roman.Li@xxxxxxx>; Hung, Alex
> <Alex.Hung@xxxxxxx>; Wentland, Harry <Harry.Wentland@xxxxxxx>; Hamza
> Mahfooz <hamza.mahfooz@xxxxxxx>; Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> 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@xxxxxxx>
> Cc: Tom Chung <chiahsuan.chung@xxxxxxx>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx>
> Cc: Roman Li <roman.li@xxxxxxx>
> Cc: Alex Hung <alex.hung@xxxxxxx>
> Cc: Aurabindo Pillai <aurabindo.pillai@xxxxxxx>
> Cc: Harry Wentland <harry.wentland@xxxxxxx>
> Cc: Hamza Mahfooz <hamza.mahfooz@xxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> ---
> 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
