Re: [PATCH v4 5/8] drm/bridge: pass down IRQ_HPD to the drivers

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Modifying connector EDID properties without holding `mode_config.mutex` during asynchronous hotplug notifications.
--

commit 6e60e13be304386d0a94f5ceade83e44677e2a2a
Author: Dmitry Baryshkov <[email protected]>

drm/bridge: pass down IRQ_HPD to the drivers

The DisplayPort standard defines a special kind of HPD events called IRQ_HPD.
These events are used to notify DP Source about the events on the Sink side.
This commit passes down the extra status to the bridge drivers via the
hpd_notify() callback.

> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> index 9427cc2358ae1..8cb17bd0e2387 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
> @@ -429,7 +429,8 @@ static const struct drm_edid *lt9611uxc_bridge_edid_read(struct drm_bridge *brid
>  
>  static void lt9611uxc_bridge_hpd_notify(struct drm_bridge *bridge,
>  					struct drm_connector *connector,
> -					enum drm_connector_status status)
> +					enum drm_connector_status status,
> +					enum drm_connector_status_extra extra_status)
>  {
>  	const struct drm_edid *drm_edid;

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential locking
issue here in lt9611uxc_bridge_hpd_notify().

This function calls drm_edid_connector_update(), which modifies
connector->edid_blob_ptr. According to DRM documentation, edid_blob_ptr access
and modification via drm_property_replace_global_blob must be protected by
mode_config.mutex.

However, when the hpd_notify pipeline is triggered asynchronously via
drm_bridge_connector_handle_hpd() in drm_bridge_connector.c, the caller
intentionally drops the lock prior to executing the callbacks:

drm_bridge_connector_handle_hpd() {
    ...
    mutex_unlock(&dev->mode_config.mutex);
    drm_bridge_connector_hpd_notify(connector, status, extra_status);
    ...
}

Could this lead to a data race on connector->edid_blob_ptr if hotplug events
occur concurrently with userspace queries or modesets, potentially causing a
double-free or use-after-free of the blob data?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic