Re: [PATCH 16/37] drm/display: bridge-connector: move per-connector fields to the dynamic connector

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] HPD and OOB hotplug events are misrouted to the newest dynamic connector and can cause a NULL pointer dereference.
- [High] Use-After-Free due to drmm_ CEC registration for dynamically freed connectors.
- [Medium] Memory leak of bridge references on error path in drm_bridge_connector_add_connector().
--

commit 2da47a06a9f55cba5481389b325a75953c4b3912
Author: Luca Ceresoli <luca.ceresoli@bootlin.com>

drm/display: bridge-connector: move per-connector fields to the dynamic connector

This commit moves various bridge pointers and the hdmi_funcs pointer from
struct drm_bridge_connector to struct drm_bridge_connector_dynconn. This
ensures these fields share the same allocation and lifetime as the
dynamically-created connector they reference, in preparation for bridge
hotplugging support.

> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -225,7 +228,9 @@ static void drm_bridge_connector_enable_hpd(struct drm_connector *connector)
>  {
>  	struct drm_bridge_connector *bridge_connector =
>  		to_drm_bridge_connector(connector);
> -	struct drm_bridge *hpd = bridge_connector->bridge_hpd;
> +	struct drm_bridge_connector_dynconn *dynconn =
> +		to_drm_bridge_connector_dynconn(connector);
> +	struct drm_bridge *hpd = dynconn->bridge_hpd;
>  
>  	if (hpd)
>  		drm_bridge_hpd_enable(hpd, drm_bridge_connector_hpd_cb,
>  				      bridge_connector);
>  }

Does registering bridge_connector as the callback data here cause hotplug
detection events to be misrouted? 

When an event fires, the handler resolves the target via
&bridge_connector->dynconn->connector. If multiple dynamic connectors are
created concurrently, bridge_connector->dynconn will point to the most
recently created one, potentially corrupting the state of a newer connector.

Additionally, if bridge_connector->dynconn has been set to NULL (such as
after a hot-unplug), could this path trigger a NULL pointer dereference when
the callback accesses connector->status?

[ ... ]

> @@ -320,7 +325,7 @@ static void drm_bridge_connector_dynconn_destroy(struct drm_connector *connector
>  		container_of(connector, struct drm_bridge_connector_dynconn, connector);
>  
>  	drm_connector_cleanup(connector);
> -	drm_bridge_connector_put_bridges(dynconn->bridge_connector);
> +	drm_bridge_connector_put_bridges(dynconn);
>  	kfree(dynconn);
>  }

Since drm_bridge_connector_init_hdmi_audio_cec() registers components using
drmm_connector_hdmi_cec_notifier_register() and
drmm_connector_hdmi_cec_register(), the cleanup actions are deferred and
bound to the global drm_device (connector->dev). 

Because the connector is explicitly freed here via kfree(dynconn), will
the delayed drmm_ actions dereference the freed connector pointer when the
drm_device is eventually torn down, leading to a use-after-free regression?

[ ... ]

> @@ -841,26 +845,26 @@ static int drm_bridge_connector_get_bridges(struct drm_bridge_connector_dynconn
>  		 * Ensure the last bridge declares OP_EDID or OP_MODES or both.
>  		 */
>  		if (bridge->ops & DRM_BRIDGE_OP_EDID || bridge->ops & DRM_BRIDGE_OP_MODES) {
> -			drm_bridge_put(bridge_connector->bridge_edid);
> -			bridge_connector->bridge_edid = NULL;
> -			drm_bridge_put(bridge_connector->bridge_modes);
> -			bridge_connector->bridge_modes = NULL;
> +			drm_bridge_put(dynconn->bridge_edid);
> +			dynconn->bridge_edid = NULL;
> +			drm_bridge_put(dynconn->bridge_modes);
> +			dynconn->bridge_modes = NULL;
>  
>  			if (bridge->ops & DRM_BRIDGE_OP_EDID)
> -				bridge_connector->bridge_edid = drm_bridge_get(bridge);
> +				dynconn->bridge_edid = drm_bridge_get(bridge);

When this function populates these bridge references in dynconn, if it later
encounters an error (like returning -EINVAL for missing callbacks), the
caller drm_bridge_connector_add_connector() will free dynconn without calling
drm_bridge_connector_put_bridges(). 

Could this regression result in leaking these newly acquired bridge references
on the error path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-drm-bridge-hotplug-v1-0-45e2bdb3dfb4@bootlin.com?part=16