Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Unprotected double-read of `hdmi->curr_conn` causes a Time-Of-Check to Time-Of-Use (TOCTOU) data race, leading to a potential NULL pointer dereference. --
commit 8041d212a61ef15cbf004795e5f7dc2a8f5f69ad Author: Cristian Ciocaltea <[email protected]> drm/bridge: dw-hdmi-qp: Provide dw_hdmi_qp_hpd_notify() helper This commit introduces the dw_hdmi_qp_hpd_notify() helper to allow platform drivers to report hot-plug detection events directly to the affected connector. It ensures the detect context is invoked on reconnection events by using drm_connector_helper_hpd_irq_event() when the display pipeline is active. > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c > index 07b274f76e53e..4d41ad7319a58 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c [ ... ] > @@ -1429,6 +1430,36 @@ struct dw_hdmi_qp *dw_hdmi_qp_bind(struct > platform_device *pdev, > } > EXPORT_SYMBOL_GPL(dw_hdmi_qp_bind); > > +/** > + * dw_hdmi_qp_hpd_notify() - Notify a hot-plug detection event [ ... ] > +void dw_hdmi_qp_hpd_notify(struct dw_hdmi_qp *hdmi) > +{ > + enum drm_connector_status status; > + > + status = hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data); > + dev_dbg(hdmi->dev, "%s status=%d\n", __func__, status); > + > + /* > + * When the display pipeline has been already active, switch to > + * drm_connector_helper_hpd_irq_event() to ensure .detect_ctx() > + * gets invoked, i.e. via drm_helper_probe_detect(), because > + * drm_bridge_hpd_notify() defers to a delayed hotplug path in > + * this case. > + */ > + if (hdmi->curr_conn && status == connector_status_connected) > + drm_connector_helper_hpd_irq_event(hdmi->curr_conn); [Severity: High] Could this double-read of hdmi->curr_conn lead to a NULL pointer dereference due to a Time-Of-Check to Time-Of-Use race? If an HPD interrupt or workqueue executes dw_hdmi_qp_hpd_notify() concurrently with dw_hdmi_qp_bridge_atomic_disable() during a modeset, could the modeset thread clear hdmi->curr_conn locklessly? Thread 1 (HPD notify): if (hdmi->curr_conn && ...) // Evaluates to true Thread 2 (Modeset disable): dw_hdmi_qp_bridge_atomic_disable() { ... hdmi->curr_conn = NULL; } Thread 1 (HPD notify resumes): drm_connector_helper_hpd_irq_event(hdmi->curr_conn); // Passes NULL If the compiler emits a second read of the pointer for the argument, it would pass NULL to drm_connector_helper_hpd_irq_event(), which unconditionally dereferences the connector pointer. Would it be safer to use READ_ONCE() or a local variable to cache the connector pointer before checking and passing it? > + else > + drm_bridge_hpd_notify(&hdmi->bridge, status); > +} > +EXPORT_SYMBOL_GPL(dw_hdmi_qp_hpd_notify); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=20
