Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] In `drm_sysfb_plane_helper_atomic_disable()`, using the new plane state's `dst` rectangle allows an attacker to supply arbitrary, unclipped coordinates, leading to out-of-bounds writes of zeroes in kernel or I/O memory. -- --- Patch [1]: [PATCH] drm/sysfb: Fix atomic_disable to use dst coordinates instead of src --- commit b1c860517627fd82ea2b305a0494871f248ccba6 Author: [email protected] <[email protected]> drm/sysfb: Fix atomic_disable to use dst coordinates instead of src This patch updates drm_sysfb_plane_helper_atomic_disable() to use the plane_state->dst coordinates for constructing the clear rectangle. This makes the behavior consistent with drm_sysfb_plane_helper_atomic_update() which already uses the destination rectangle computed by the DRM core. > diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > index d2de29caf89eb..18466de886aaa 100644 > --- a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > +++ b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > @@ -380,14 +380,10 @@ void drm_sysfb_plane_helper_atomic_disable(struct > drm_plane *plane, > void __iomem *dst_vmap = dst.vaddr_iomem; /* TODO: Use mapping > abstraction */ > unsigned int dst_pitch = sysfb->fb_pitch; > const struct drm_format_info *dst_format = sysfb->fb_format; > - struct drm_rect dst_clip; > + struct drm_rect dst_clip = plane_state->dst; [Severity: Critical] This is a pre-existing issue, but does using the new plane state's dst rectangle allow userspace to supply arbitrary, unclipped coordinates? If userspace with DRM_MASTER privilege issues an atomic modeset with FB_ID = 0 (disabling the plane) while supplying arbitrary CRTC_X and CRTC_Y properties, drm_atomic_helper_check_plane_state() detects !fb and returns early. This bypasses coordinate bounds checking and clipping, leaving plane_state->dst set to the unclipped, userspace-controlled values. Then, drm_sysfb_plane_helper_atomic_disable() calculates a memory offset using drm_fb_clip_offset(dst_pitch, dst_format, &dst_clip), which translates directly to the userspace-supplied coordinates. This offset is used in a loop calling memset_io(), which could lead to out-of-bounds physical or I/O memory writes of zeroes. > unsigned long lines, linepixels, i; > int idx; > > - drm_rect_init(&dst_clip, > - plane_state->src_x >> 16, plane_state->src_y >> 16, > - plane_state->src_w >> 16, plane_state->src_h >> 16); > - > lines = drm_rect_height(&dst_clip); > linepixels = drm_rect_width(&dst_clip); > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
