Re: [PATCH] drm/meson: constrain the DW-HDMI regmap range

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The threaded IRQ handler `dw_hdmi_top_thread_irq` can execute before the `hdmi` and `bridge` pointers are initialized, leading to a NULL pointer dereference.
--

--- Patch [1]: [PATCH] drm/meson: constrain the DW-HDMI regmap range ---
commit f8ff68e6f372ddb9f5ca83893e7f1b1b21342d72
Author: Zinan Zhou <zinan@mieulab.com>

drm/meson: constrain the DW-HDMI regmap range

The DesignWare HDMI regmap declares 0x10000 as its inclusive maximum
register. A regmap traversal can consequently read one byte past the
directly mapped G12A 64 KiB resource. It also treats the Meson TOP window
from offset 0x8000 as DWC registers. On the older indirect interface, the
address is truncated to 16 bits and aliases register zero.

Limit the regmap to 0x7e12, the last register defined by the supported
Synopsys HDMI controller. This also keeps register iteration below the
Meson TOP register window at offset 0x8000.

> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index fef1702acb145..1004108fb7ca2 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c

[ ... ]

> @@ -564,7 +566,7 @@ static const struct regmap_config meson_dw_hdmi_regmap_config = {
>  	.val_bits = 8,
>  	.reg_read = meson_dw_hdmi_reg_read,
>  	.reg_write = meson_dw_hdmi_reg_write,
> -	.max_register = 0x10000,
> +	.max_register = HDMITX_DWC_MAX_REGISTER,
>  	.fast_io = true,
>  };

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this file I
noticed a potential race condition during initialization.

In meson_dw_hdmi_bind(), the IRQ handler is registered via
devm_request_threaded_irq() before dw_hdmi->hdmi and dw_hdmi->bridge are
initialized:

drivers/gpu/drm/meson/meson_dw_hdmi.c:meson_dw_hdmi_bind() {
    ...
    ret = devm_request_threaded_irq(dev, irq, dw_hdmi_top_irq,
                                    dw_hdmi_top_thread_irq, IRQF_SHARED,
                                    "dw_hdmi_top_irq", meson_dw_hdmi);
    ...
    meson_dw_hdmi->hdmi = dw_hdmi_probe(pdev, &meson_dw_hdmi->dw_plat_data);
    ...
    meson_dw_hdmi->bridge = of_drm_find_and_get_bridge(pdev->dev.of_node);
    ...
}

The driver then calls dw_hdmi_probe(), which unmasks the HPD interrupts at
the hardware level.

If an HPD interrupt fires immediately (e.g., if a cable is already connected
or the line bounces), can dw_hdmi_top_thread_irq() execute concurrently and
cause a NULL pointer dereference?

drivers/gpu/drm/meson/meson_dw_hdmi.c:dw_hdmi_top_thread_irq() {
    ...
    dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected,
                           hpd_connected);

    drm_helper_hpd_irq_event(dw_hdmi->bridge->dev);
    ...
}

Since dw_hdmi_probe() has not yet returned, wouldn't dw_hdmi->hdmi and
dw_hdmi->bridge still be NULL at this point, causing a crash when
dereferencing them?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718090710.70534-1-zinan@mieulab.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic