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

[Yongxing Mou <yongxing.mou@oss.qualcomm.com>]

On 6/9/2026 6:02 PM, sashiko-bot@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.mou@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;
>