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