Re: [PATCH 22/37] drm/bridge: initialize chain_node list head on detach and attach errors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Data race and NULL pointer dereference in drm_bridge_attach() error path during dynamic hotplug
--

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

drm/bridge: initialize chain_node list head on detach and attach errors

This commit updates drm_bridge_detach() and the error path of
drm_bridge_attach() to use list_del_init() instead of list_del(). This
ensures the bridge chain_node list head is clearly empty when detached,
preventing double-detach issues during hotplug operations.

> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index f179e00745b7c..5b89168411aa0 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -598,7 +598,7 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>  	bridge->dev = NULL;
>  	bridge->encoder = NULL;
>  	mutex_lock(&encoder->bridge_chain_mutex);
> -	list_del(&bridge->chain_node);
> +	list_del_init(&bridge->chain_node);
>  	mutex_unlock(&encoder->bridge_chain_mutex);

This is an existing issue not introduced by this patch, but since dynamic
bridge hotplugging support is being introduced, can this sequence cause a NULL
pointer dereference?

In this error path, bridge->dev and bridge->encoder are cleared to NULL
outside the encoder->bridge_chain_mutex critical section, while the bridge
is still linked into the encoder->bridge_chain.

If drm_bridge_attach() is executed at runtime and fails, a concurrent
userspace atomic modeset iterating over encoder->bridge_chain could observe
the bridge with bridge->encoder == NULL. Drivers typically use this to
fetch the connector:

connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);

If bridge->encoder is NULL, this returns a NULL connector, which could lead
to a crash when calling drm_connector_index(connector). 

Should the field assignments be moved inside the mutex-protected block, or
after the bridge is removed from the list?

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