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

Pre-existing issues:
- [High] Uninitialized wait queue usage leads to NULL pointer dereference.
- [High] Missing drvdata initialization before component bind leads to NULL 
pointer dereference.
--

commit 1c9b9cfe82b003fd952c66b9a2f9e8fa689477ee
Author: Luca Leonardo Scorcia <[email protected]>

drm/mediatek: dsi: Add compatible for mt8167-dsi

This commit adds the mediatek,mt8167-dsi compatible string to the MediaTek
DSI controller driver. It allows the driver to probe on MT8167 hardware using
the existing MT2701 driver data.

Link: 
https://patchwork.kernel.org/project/dri-devel/patch/[email protected]/

> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c 
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 0e2bcd5f67b76..b04936d4511b4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1307,6 +1307,7 @@ static const struct mtk_dsi_driver_data 
> mt8188_dsi_driver_data = {
>  
>  static const struct of_device_id mtk_dsi_of_match[] = {
>       { .compatible = "mediatek,mt2701-dsi", .data = &mt2701_dsi_driver_data 
> },
> +     { .compatible = "mediatek,mt8167-dsi", .data = &mt2701_dsi_driver_data 
> },

[Severity: High]
This is a pre-existing issue, but enabling this driver on new hardware might
expose an uninitialized wait queue usage in drivers/gpu/drm/mediatek/mtk_dsi.c
that leads to a NULL pointer dereference.

In mtk_dsi_probe(), mipi_dsi_host_register() and devm_request_irq() are called
before the wait queue is initialized:

        ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq, ...);
        if (ret) {
            ...
        }

        init_waitqueue_head(&dsi->irq_wait_queue);

If a child panel driver probes synchronously when mipi_dsi_host_register() is
called and sends a DSI command (for instance, to read the panel ID), it
triggers mtk_dsi_host_transfer(). This calls mtk_dsi_wait_for_irq_done(),
which uses wait_event_interruptible_timeout() on the zero-initialized wait
queue. This dereferences the NULL next pointer of the wait queue head, causing
a crash.

Alternatively, if an interrupt fires early, mtk_dsi_irq() calls
wake_up_interruptible() on the same uninitialized wait queue.

Could the initialization of dsi->irq_wait_queue be moved before
mipi_dsi_host_register() and devm_request_irq() to prevent this?

[Severity: High]
This isn't a bug introduced by this patch, but missing drvdata initialization
before the component bind can also lead to a NULL pointer dereference.

In mtk_dsi_probe(), the DSI host is registered before drvdata is set:

        ret = mipi_dsi_host_register(&dsi->host);
        ...
        platform_set_drvdata(pdev, dsi);

Registering the DSI host causes child devices to be probed synchronously. The
child's probe function calls mipi_dsi_attach(), which invokes
mtk_dsi_host_attach(). This function calls component_add(). If mtk_dsi is the
last component to register, it triggers the DRM master to bind all components,
invoking mtk_dsi_bind().

However, mtk_dsi_bind() tries to use drvdata:

        struct mtk_dsi *dsi = dev_get_drvdata(dev);

        ret = mtk_dsi_encoder_init(drm, dsi);

Because platform_set_drvdata() hasn't been called yet, dev_get_drvdata()
returns NULL, leading to a crash in mtk_dsi_encoder_init().

Would it be possible to call platform_set_drvdata() before
mipi_dsi_host_register() to avoid this race?

>       { .compatible = "mediatek,mt8173-dsi", .data = &mt8173_dsi_driver_data 
> },
>       { .compatible = "mediatek,mt8183-dsi", .data = &mt8183_dsi_driver_data 
> },
>       { .compatible = "mediatek,mt8186-dsi", .data = &mt8186_dsi_driver_data 
> },

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to