Re: [PATCH v7 25/35] drm/bridge_connector: suppress hotplug for IRQ_HPD without status changes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm: missing hotplug event due to TOCTOU race in HPD handler
--

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

drm/bridge_connector: suppress hotplug for IRQ_HPD without status changes

This commit suppresses hotplug events for DP IRQ_HPD notifications if there
are no connection status changes. It checks the actual connector status
after handling the notification using the bridge detect callback. Hotplug
events are only sent if the connection state transitions, preventing
unnecessary reprobes.

> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -159,15 +159,28 @@ static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *drm_bri
[ ... ]
>  	mutex_lock(&dev->mode_config.mutex);
> -	connector->status = status;
> +	old_status = connector->status;
>  	mutex_unlock(&dev->mode_config.mutex);
>  
>  	drm_bridge_connector_hpd_notify(connector, status, event);
>  
> -	drm_kms_helper_connector_hotplug_event(connector);
> +	mutex_lock(&dev->mode_config.mutex);
> +	new_status = detect ? detect->funcs->detect(detect, connector) : status;
> +	connector->status = new_status;
> +	changed = new_status != old_status;

[Severity: Medium]
Could this logic cause a missing hotplug event due to a race condition?

If old_status = A is read, the lock is dropped. During the execution of
drm_bridge_connector_hpd_notify(), a concurrent ioctl (like
DRM_IOCTL_MODE_GETCONNECTOR) might acquire the lock, probe the hardware,
update connector->status to B, and return state B to userspace.

If the hardware then changes back to A before this HPD handler reacquires
the lock, the detect call could return A. The new_status becomes A, and the
code calculates changed = (A != A), which is false, suppressing the hotplug
event.

In this scenario, userspace tracks state B, but the kernel tracks state A,
and userspace never receives the hotplug event to notify it of the
transition back to A.

Should old_status be read within the second critical section just before
calling the detect function, so that we compare against the most recent
state?

> +	mutex_unlock(&dev->mode_config.mutex);
> +
> +	if (changed || level_hpd)
> +		drm_kms_helper_connector_hotplug_event(connector);
>  }

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