Re: [PATCH v9 04/61] drm/display: bridge_connector: Convert to drmm_connector_hdmi_init()

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

Pre-existing issues:
- [High] Lockless removal of bridge from `bridge_lingering_list` can corrupt the global list.
--

--- Patch [4]: [PATCH v9 04/61] drm/display: bridge_connector: Convert to drmm_connector_hdmi_init() ---
commit d79fbb96aa85834c574a6211898049f6cf97c1a6
Author: Cristian Ciocaltea <[email protected]>

drm/display: bridge_connector: Convert to drmm_connector_hdmi_init()

Switch drm_bridge_connector to the new drmm_connector_hdmi_init()
signature. The vendor, product, supported_formats and max_bpc values
now live in struct drm_connector_hdmi_funcs instead of being passed as
separate arguments. Introduce supported_hdmi_ver and max_tmds_char_rate to
struct drm_bridge to let bridges declare their capabilities.

> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 83f1809a5d376..afaae272347c0 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -460,9 +460,13 @@ void drm_bridge_add(struct drm_bridge *bridge)

[Severity: High]
This is a pre-existing issue, but looking at drm_bridge_add() just before
this hunk, does the lockless list removal risk corrupting the global
bridge_lingering_list?

It looks like the bridge is being removed from the list without acquiring
bridge_lock:

	if (!list_empty(&bridge->list))
		list_del_init(&bridge->list);

If another thread concurrently modifies bridge_lingering_list via
drm_bridge_remove(), or iterates over it via debugfs, couldn't this
lead to list pointer corruption or a crash?

>  	mutex_init(&bridge->hpd_state_mutex);
>  	mutex_init(&bridge->hpd_mutex);
>  
> -	if (bridge->ops & DRM_BRIDGE_OP_HDMI)
> +	if (bridge->ops & DRM_BRIDGE_OP_HDMI) {
> +		if (bridge->supported_hdmi_ver == HDMI_VERSION_UNKNOWN)
> +			DRM_WARN("HDMI bridge misses supported HDMI version\n");
> +
>  		bridge->ycbcr_420_allowed = !!(bridge->supported_formats &
>  					       BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420));
> +	}
>  
>  	mutex_lock(&bridge_lock);
>  	list_add_tail(&bridge->list, &bridge_list);

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