Re: [PATCH v7 12/15] drm/msm/dp: simplify link and clock disable sequence

[Yongxing Mou]

On 6/9/2026 6:02 PM, sashiko-...@kernel.org wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] The patch introduces a Time-of-Check to Time-of-Use (TOCTOU) race by 
splitting a single `sink_count` evaluation into two unprotected checks, risking 
an inconsistent hardware disable sequence.
--

commit 4b86b22d6f1aa41eba1cece8a907cf0b3a12b55c
Author: Yongxing Mou <yongxing....@oss.qualcomm.com>

drm/msm/dp: simplify link and clock disable sequence

This patch refactors the disable sequence by pulling common steps out
of the sink_count condition block to simplify the flow.

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c

[ ... ]

@@ -716,27 +716,19 @@ static int msm_dp_display_disable(struct 
msm_dp_display_private *dp)

msm_dp_panel_disable_vsc_sdp(dp->panel); - /* dongle is still connected but sinks are disconnected */

-       if (dp->link->sink_count == 0) {

[ ... ]

+       msm_dp_ctrl_off_pixel_clk(dp->ctrl);

- /* set dongle to D3 (power off) mode */

+       /* dongle is still connected but sinks are disconnected */
+       if (dp->link->sink_count == 0)
                msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
-               msm_dp_ctrl_off_pixel_clk(dp->ctrl);
-               msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
+
+       msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
+
+       if (dp->link->sink_count == 0)

[Severity: High]
Could this double-evaluation of dp->link->sink_count introduce a race
condition?

The original code checked dp->link->sink_count == 0 exactly once. By
checking it twice, if an asynchronously processed HPD IRQ event modifies
the sink_count between the two checks, could the driver execute a
mismatched hardware disable sequence?

For instance, if sink_count changes from 0 to 1 after the first check,
the driver would perform msm_dp_link_psm_config(), but then take the
else branch and call msm_dp_display_host_phy_exit() below. This would
leave the PHY completely powered down, potentially breaking the ability
to detect future dongle disconnects.

Would it be safer to read sink_count once into a local variable and
evaluate the local variable for both conditions?

No. Here is safe. No one will update it when link off.

+               /* re-init the PHY so that we can listen to Dongle disconnect */
                msm_dp_ctrl_reinit_phy(dp->ctrl);
-       } else {
-               /*
-                * unplugged interrupt
-                * dongle unplugged out of DUT
-                */
-               msm_dp_ctrl_off_pixel_clk(dp->ctrl);
-               msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
+       else
                msm_dp_display_host_phy_exit(dp);
-       }

msm_dp_display->power_on = false;