Re: [PATCH v4 04/12] drm/mediatek: mtk_dp: Fix hdmi codec and phy driver unregistration

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The hardware interrupt is not disabled during driver unbind (`mtk_dp_remove`), allowing the IRQ handler and thread to execute while resources are being destroyed, leading to use-after-free, timer re-arming, and bus errors.
- [High] The ALSA `get_eld` callback can dereference a NULL `mtk_dp->conn` if called between a hotplug event and a DRM modeset.
--

--- Patch [4]: [PATCH v4 04/12] drm/mediatek: mtk_dp: Fix hdmi codec and phy driver unregistration ---
commit 6ce33bac50c12bbd077cc2e659b77509b20cd720
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Subject: drm/mediatek: mtk_dp: Fix hdmi codec and phy driver unregistration

This commit converts the unregistration of the HDMI Codec and DisplayPort PHY
platform devices to use devm actions. This ensures they are properly cleaned
up even during probe deferrals or failures.

> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index 0d0507890d993..8399a6b411689 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
[ ... ]
> @@ -2680,18 +2687,28 @@ static int mtk_dp_register_audio_driver(struct device *dev)
>  		.data = mtk_dp,
>  		.no_capture_mute = 1,
>  	};
> +	int ret;
>  
>  	mtk_dp->audio_pdev = platform_device_register_data(dev,
>  							   HDMI_CODEC_DRV_NAME,
>  							   PLATFORM_DEVID_AUTO,
>  							   &codec_data,
>  							   sizeof(codec_data));

[Severity: High]
This is a pre-existing issue, but does the ALSA get_eld callback have a
potential NULL pointer dereference race with hotplug events?

When a display is plugged in, mtk_dp_hpd_event_thread() unconditionally sets
the enabled flag before the DRM core populates mtk_dp->conn during a modeset:

mtk_dp_hpd_event_thread()
    ...
    mtk_dp->enabled = true;

If userspace or the sound subsystem queries the ELD via mtk_dp_audio_get_eld()
in the window between the hotplug event and the DRM modeset, it seems we
dereference mtk_dp->conn without checking if it is NULL:

mtk_dp_audio_get_eld()
    if (mtk_dp->enabled)
        memcpy(buf, mtk_dp->conn->eld, len);

Could this lead to a crash if a local user probes ALSA mixers before the
modeset completes?

[ ... ]
> @@ -2870,9 +2890,6 @@ static void mtk_dp_remove(struct platform_device *pdev)
>  	pm_runtime_disable(&pdev->dev);
>  	if (mtk_dp->data->bridge_type != DRM_MODE_CONNECTOR_eDP)
>  		timer_delete_sync(&mtk_dp->debounce_timer);
> -	platform_device_unregister(mtk_dp->phy_dev);
> -	if (mtk_dp->audio_pdev)
> -		platform_device_unregister(mtk_dp->audio_pdev);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but since we are modifying driver
removal, is there a risk of the hardware interrupt firing during or after
mtk_dp_remove()?

The hardware interrupt is managed by devres and remains active until after
mtk_dp_remove() returns. Since pm_runtime_disable() is called here, the
device power domain might be turned off.

If a hotplug interrupt fires during this window, mtk_dp_hpd_event() will
attempt to read hardware registers:

mtk_dp_hpd_event()
    u32 irq_status = mtk_dp_swirq_get_clear(mtk_dp) |
                     mtk_dp_hwirq_get_clear(mtk_dp);

Could this cause a bus error or crash if the power domain is already off?

Furthermore, if the interrupt handler schedules the IRQ thread,
mtk_dp_hpd_event_thread() could re-arm the debounce timer or attempt to
access the PHY during training after it has been unregistered by devres,
leading to a use-after-free:

mtk_dp_hpd_event_thread()
    /* Training */
    ret = mtk_dp_training(mtk_dp);

Should the IRQ be explicitly disabled in mtk_dp_remove() before deleting
the timers and disabling pm runtime?

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