Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Integer underflow in plane dimensions when a plane is fully clipped, causing hardware misconfiguration. --
commit dbd00e4ed77401b79cffe99deaf03ef1ee191363 Author: Liu Ying <[email protected]> drm/imx: dc-fu: Fix dimensions This commit fixes an off-by-one issue in LINEWIDTH, LINECOUNT, FRAMEWIDTH and FRAMEHEIGHT macro definitions used to set fetchunit dimensions. > diff --git a/drivers/gpu/drm/imx/dc/dc-fu.c b/drivers/gpu/drm/imx/dc/dc-fu.c > index 1d8f74babef8a..2e197b3344bd1 100644 > --- a/drivers/gpu/drm/imx/dc/dc-fu.c > +++ b/drivers/gpu/drm/imx/dc/dc-fu.c > @@ -31,8 +31,8 @@ > #define STRIDE(x) FIELD_PREP(STRIDE_MASK, (x) - 1) > > /* SOURCEBUFFERDIMENSION */ > -#define LINECOUNT(x) FIELD_PREP(GENMASK(29, 16), (x)) > -#define LINEWIDTH(x) FIELD_PREP(GENMASK(13, 0), (x)) > +#define LINECOUNT(x) FIELD_PREP(GENMASK(29, 16), (x) - 1) > +#define LINEWIDTH(x) FIELD_PREP(GENMASK(13, 0), (x) - 1) [Severity: High] Could this unconditional subtraction cause an integer underflow when a plane is fully clipped? When an unprivileged userspace application moves a DRM plane completely off-screen, drm_atomic_helper_check_plane_state() sets plane_state->visible = false and clips the src and dst rectangles such that their width and height become 0 or negative. Because dc_plane_atomic_update() lacks an early return for !new_state->visible, it appears to extract src_w and src_h and pass them to these macros: drivers/gpu/drm/imx/dc/dc-plane.c:dc_plane_atomic_update() { ... src_w = drm_rect_width(&new_state->src) >> 16; src_h = drm_rect_height(&new_state->src) >> 16; ... fu_ops->set_src_buf_dimensions(fu, DC_FETCHUNIT_FRAC0, src_w, src_h); fu_ops->set_framedimensions(fu, src_w, src_h); ... } Because (x) - 1 underflows for x <= 0, FIELD_PREP() would apply the bitmask and result in the maximum possible field value (16383) being written to the hardware registers. Will this cause out-of-bounds DMA fetches leading to IOMMU faults or system lockups? > diff --git a/drivers/gpu/drm/imx/dc/dc-fu.h b/drivers/gpu/drm/imx/dc/dc-fu.h > index f678de3ca8c0a..cc93317e30013 100644 > --- a/drivers/gpu/drm/imx/dc/dc-fu.h > +++ b/drivers/gpu/drm/imx/dc/dc-fu.h > @@ -38,8 +38,8 @@ > #define YUVCONVERSIONMODE(x) FIELD_PREP(YUVCONVERSIONMODE_MASK, (x)) > > /* FRAMEDIMENSIONS */ > -#define FRAMEHEIGHT(x) FIELD_PREP(GENMASK(29, 16), (x)) > -#define FRAMEWIDTH(x) FIELD_PREP(GENMASK(13, 0), (x)) > +#define FRAMEHEIGHT(x) FIELD_PREP(GENMASK(29, 16), (x) > - 1) > +#define FRAMEWIDTH(x) FIELD_PREP(GENMASK(13, 0), (x) > - 1) [Severity: High] Similarly, will these frame dimension macros also underflow to the maximum field width when height or width is 0 or negative? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
