Re: [PATCH v8 20/39] drm/bridge: dw-hdmi-qp: Provide dw_hdmi_qp_hpd_notify() helper

[sashiko-bot@kernel.org]

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 <cristian.ciocaltea@collabora.com>

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/20260702-dw-hdmi-qp-scramb-v8-0-d79890d00b6a@collabora.com?part=20