Re: [PATCH v4 02/10] drm/atomic-helpers: Evaluate plane damage after atomic_check

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Modifying `ignore_damage_clips` after `plane->atomic_check` runs invalidates resource allocations based on partial damage clips, leading to heap buffer overflows.
--

commit 395f91f063087b27412023cba7e5d824633a9392
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/atomic-helpers: Evaluate plane damage after atomic_check

This commit evaluates damage clips after running the atomic_check helpers.
It adds a loop calling drm_atomic_helper_check_plane_damage() after the
driver's plane and CRTC atomic_check hooks have executed. This allows the
damage handling to account for full modesets triggered by the check hooks.

> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 51f39edc31edc..4c37299e8ccb7 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
[ ... ]
> @@ -1065,6 +1065,10 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
>  		}
>  	}
>  
> +	for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
> +		drm_atomic_helper_check_plane_damage(state, new_plane_state);
> +	}
> +

[Severity: High]
Does modifying ignore_damage_clips after plane->atomic_check has already
run invalidate resource allocations in drivers that use partial damage clips?

For example, in drivers/gpu/drm/tiny/appletbdrm.c:appletbdrm_primary_plane_helper_atomic_check(),
the driver sizes a buffer precisely to the damage clips evaluated during
atomic_check:

	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, new_plane_state);
	drm_atomic_for_each_plane_damage(&iter, &damage) {
		frames_size += struct_size((struct appletbdrm_frame *)0, buf, rect_size(&damage));
	}
	...
	appletbdrm_state->request = kvzalloc(request_size, GFP_KERNEL);

If a modeset is triggered during the check hooks and this second loop sets
ignore_damage_clips to true, won't drm_atomic_helper_damage_iter_init() later
return a full-screen damage clip during atomic_update? Could this cause the
driver to copy full-screen damage into the buffer allocated only for partial
damage, resulting in a heap buffer overflow?

I understand this is explicitly acknowledged and fixed later in the patch
series by commit 72afced3e1b6baa294269874c7c4d9d05230889c ('drm/appletbdrm:
Allocate request/response buffers in begin_fb_access').

>  	return ret;
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_check_planes);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530185716.65688-1-tzimmermann@suse.de?part=2