Re: [PATCH v7 34/35] drm/msm/dp: order IRQ HPD handling with plug state changes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/msm/dp: TOCTOU race in IRQ_HPD handling causes pm_runtime leak for SST/eDP
--

commit 99b95a07a6fbe8a19b13f048313ddac554ebf859
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>

drm/msm/dp: order IRQ HPD handling with plug state changes

This commit orders IRQ HPD handling with plug state changes. It ensures
the link is initialized via the plug path before handling the IRQ, and
ignores stale IRQs if an unplug event is also present.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index ed0b93e3bd08f..94446bcf9448a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -1818,6 +1821,9 @@ void msm_dp_bridge_hpd_notify(struct drm_bridge *bridge,
>  	if (status == connector_status_connected) {
>  		if (hpd_link_status == ISR_IRQ_HPD_PULSE_COUNT ||
>  		    event == DRM_CONNECTOR_DP_IRQ_HPD) {
> +			if (!dp->plugged)
> +				msm_dp_hpd_plug_handle(dp);
> +
[Severity: Medium]
Can this lockless read of dp->plugged lead to a Time-Of-Check to Time-Of-Use
race with concurrent threads (like userspace detect or modeset)?

If msm_dp_bridge_hpd_notify() checks if (!dp->plugged) here and evaluates it
as true, it will prepare to call msm_dp_hpd_plug_handle(). Concurrently, if
another thread (e.g., userspace polling via msm_dp_bridge_detect) takes the
plugged_lock, gets a PM reference, and sets dp->plugged = true, this thread
will then call msm_dp_hpd_plug_handle() anyway.

Inside msm_dp_hpd_plug_handle(), we have:

    guard(mutex)(&dp->plugged_lock);
    if (dp->plugged && dp->msm_dp_display.mst_active)
        return 0;
    
    ret = pm_runtime_resume_and_get(&pdev->dev);

Because mst_active is false for SST/eDP, it fails to abort when already
plugged by the racing thread, proceeding to acquire a second PM reference.
This can result in a permanent pm_runtime reference leak, preventing the DP
controller from suspending, as unplug_handle() will only drop one reference.

>  			msm_dp_irq_hpd_handle(dp);
>  		} else if (hpd_link_status == ISR_HPD_REPLUG_COUNT) {
>  			msm_dp_hpd_unplug_handle(dp);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-msm-dp-mst-v7-0-4b81473185e7@oss.qualcomm.com?part=34