[PATCH] drm/amd/display: perform a bounds check before filling dirty rectangles

[Leo Li]

On 6/21/23 17:57, Hamza Mahfooz wrote:
> Currently, it is possible for us to access memory that we shouldn't.
> Since, we acquire (possibly dangling) pointers to dirty rectangles
> before doing a bounds check to make sure we can actually accommodate the
> number of dirty rectangles userspace has requested to fill. This issue
> is especially evident if a compositor requests both MPO and damage clips
> at the same time, in which case I have observed a soft-hang. So, to
> avoid this issue, perform the bounds check before filling a single dirty
> rectangle and WARN() about it, if it is ever attempted in
> fill_dc_dirty_rect().
> 
> Cc: stable at vger.kernel.org # 6.1+
> Fixes: 30ebe41582d1 ("drm/amd/display: add FB_DAMAGE_CLIPS support")
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz at amd.com>

Reviewed-by: Leo Li <sunpeng.li at amd.com>

Thanks!

> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)
> 
> 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 64b8dcf8dbda..66bb03d503ea 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5065,11 +5065,7 @@ static inline void fill_dc_dirty_rect(struct drm_plane *plane,
>   				      s32 y, s32 width, s32 height,
>   				      int *i, bool ffu)
>   {
> -	if (*i > DC_MAX_DIRTY_RECTS)
> -		return;
> -
> -	if (*i == DC_MAX_DIRTY_RECTS)
> -		goto out;
> +	WARN_ON(*i >= DC_MAX_DIRTY_RECTS);
>   
>   	dirty_rect->x = x;
>   	dirty_rect->y = y;
> @@ -5085,7 +5081,6 @@ static inline void fill_dc_dirty_rect(struct drm_plane *plane,
>   			"[PLANE:%d] PSR SU dirty rect at (%d, %d) size (%d, %d)",
>   			plane->base.id, x, y, width, height);
>   
> -out:
>   	(*i)++;
>   }
>   
> @@ -5172,6 +5167,9 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
>   
>   	*dirty_regions_changed = bb_changed;
>   
> +	if ((num_clips + (bb_changed ? 2 : 0)) > DC_MAX_DIRTY_RECTS)
> +		goto ffu;
> +
>   	if (bb_changed) {
>   		fill_dc_dirty_rect(new_plane_state->plane, &dirty_rects[i],
>   				   new_plane_state->crtc_x,
> @@ -5201,9 +5199,6 @@ static void fill_dc_dirty_rects(struct drm_plane *plane,
>   				   new_plane_state->crtc_h, &i, false);
>   	}
>   
> -	if (i > DC_MAX_DIRTY_RECTS)
> -		goto ffu;
> -
>   	flip_addrs->dirty_rect_count = i;
>   	return;
>