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 <[email protected]> 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/[email protected]?part=25
