Re: [PATCH v6 03/13] drm/imx: dc-fu: Fix dimensions

[sashiko-bot@kernel.org]

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 <victor.liu@nxp.com>

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/20260904-imx8-dc-prefetch-v6-0-26643e3f6cb7@nxp.com?part=3