Re: [PATCH v2 06/10] drm/mediatek: mtk_dsi: Add support for MT8189

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Inaccurate link rate overestimation falsely rejects valid modes and incorrectly configures the PHY clock when DSC is enabled.

Pre-existing issues:
- [High] Silent 32-bit integer overflow when calculating DSI data rate for high-resolution panels on 32-bit architectures.
- [Medium] 32-bit signed integer overflow in `wanted_link_rate` calculation allows maliciously crafted modes to bypass the maximum link rate security check.
--

--- Patch [6]: [PATCH v2 06/10] drm/mediatek: mtk_dsi: Add support for MT8189 ---
commit 06bb023e18f2fab2efd045626f608227b5c0522f
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Subject: drm/mediatek: mtk_dsi: Add support for MT8189

This commit adds support for the DSI IP in the MT8189 SoC, which supports a
higher link rate of 2.5Gbps. It introduces a max_link_rate_mbps member to
parameterize the maximum link rate per SoC and updates the mode validation
callback to use this new parameter with 64-bit math.

> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 6a77ce8eea91f..2df2a9d5ec038 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1070,13 +1072,20 @@ mtk_dsi_bridge_mode_valid(struct drm_bridge *bridge,
>  			  const struct drm_display_mode *mode)
>  {
>  	struct mtk_dsi *dsi = bridge_to_dsi(bridge);
> +	const struct mtk_dsi_driver_data *data = dsi->driver_data;
> +	u64 wanted_link_rate, max_link_rate;
>  	int bpp;
>  
>  	bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>  	if (bpp < 0)
>  		return MODE_ERROR;
>  
> -	if (mode->clock * bpp / dsi->lanes > 1500000)
> +	wanted_link_rate = mode->clock * bpp;

[Severity: Medium]
This is a pre-existing issue, but does this calculation allow a 32-bit signed
integer overflow? Since mode->clock and bpp are both evaluated as 32-bit
signed integers, an abnormally high clock value could wrap around to a small
positive value before being zero-extended and assigned to the 64-bit
wanted_link_rate. Could this allow crafted modes to bypass the maximum link
rate check?

[Severity: High]
Does this calculation also overestimate the link rate when Display Stream
Compression (DSC) is enabled? By using the raw uncompressed bpp instead of
the compressed bit rate (dsi->dsc->bits_per_pixel), high-resolution
DSC-enabled displays might be falsely rejected as MODE_CLOCK_HIGH.

[Severity: High]
This is a pre-existing issue, but does a similar 32-bit integer overflow occur
in mtk_dsi_poweron() on 32-bit architectures? 

drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_poweron() {
    ...
    dsi->data_rate = DIV_ROUND_UP_ULL(dsi->vm.pixelclock * bit_per_pixel,
                                      dsi->lanes);
    ...
}

On 32-bit systems, pixelclock is a 32-bit unsigned long. For high-resolution
panels, the multiplication could overflow the 32-bit limit before the macro
casts the result to unsigned long long, resulting in a corrupted frequency
being sent to clk_set_rate().

Additionally, does mtk_dsi_poweron() configure the hardware DSI HS clock
using the uncompressed data rate rather than the DSC compressed data rate?
If so, could this overclock the hardware and cause it to fail when DSC is
active?

> +	max_link_rate = data->max_link_rate_mbps;
> +	max_link_rate *= dsi->lanes;
> +	max_link_rate *= KILO;
> +
> +	if (wanted_link_rate > max_link_rate)
>  		return MODE_CLOCK_HIGH;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713095837.16590-1-angelogioacchino.delregno@collabora.com?part=6