Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Out-of-bounds MMIO write in mtk_dsi_cmdq() due to unchecked msg->tx_len. - [High] Register access with disabled clocks after mtk_dsi_poweron() fails causes kernel panic. -- commit f15ebc6147af7232db3b125277a76b1726ee2a6a Author: AngeloGioacchino Del Regno <[email protected]> drm/mediatek: mtk_dsi: Add support for MT8189 This commit adds support for the DSI IP found in the MT8189 SoC. It updates the device tree match table to map the new compatible string to the DSI driver. > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index bd2d17017bd2f..08f6be311dd14 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c [ ... ] > @@ -826,6 +826,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = { > { .compatible = "mediatek,mt8188-dsi", > .data = (void *)MTK_DSI }, > + { .compatible = "mediatek,mt8189-dsi", > + .data = (void *)MTK_DSI }, [Severity: High] This isn't a bug introduced by this patch, but adding a new DSI user exposes a pre-existing issue in mtk_dsi_cmdq(). Does this code overflow the MMIO boundaries if msg->tx_len is large? In drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_cmdq(): for (i = 0; i < msg->tx_len; i++) mtk_dsi_mask(dsi, (data->reg_adv[DSI_CMDQ] + cmdq_off + i) & (~0x3U), (0xffUL << (((i + cmdq_off) & 3U) * 8U)), tx_buf[i] << (((i + cmdq_off) & 3U) * 8U)); Because msg->tx_len isn't validated against the hardware command queue size, a large payload could write past the queue's boundary and corrupt adjacent hardware registers. [Severity: High] This is a pre-existing issue, but registering a new DSI device also inherits this error handling regression. Can this sequence cause a panic if mtk_dsi_poweron() fails? In drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_bridge_atomic_pre_enable(): ret = mtk_dsi_poweron(dsi); if (ret < 0) drm_err(drm, "failed to power on dsi\n"); mtk_dsi_lane_ready(dsi); mtk_dsi_clk_hs_mode(dsi, 1); If power-on fails, the error path within mtk_dsi_poweron() disables the clocks. However, this function ignores the failure, logs an error, and unconditionally proceeds to call mtk_dsi_lane_ready(dsi) and mtk_dsi_clk_hs_mode(dsi, 1). These subsequent calls write to hardware registers while the device clocks are gated, which typically triggers a synchronous external abort. > { } > }; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
