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 <[email protected]> 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/[email protected]?part=2
