diff mbox series

[v11,12/74] drm/bridge: Fix unlocked list_del in drm_bridge_add()

Message ID 20260901-dw-hdmi-qp-scramb-v11-12-bc12954a0688@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea Sept. 1, 2026, 6:50 p.m. UTC 
When re-adding a bridge that was previously removed, drm_bridge_add()
drops it from bridge_lingering_list without holding bridge_lock.

Both bridge_list and bridge_lingering_list are protected by bridge_lock,
as they are concurrently modified by drm_bridge_remove() and
__drm_bridge_free(), and walked by the debugfs 'bridges' file.  Running
the list_empty() test and the list_del_init() outside of the lock may
therefore corrupt either list.

Perform both under bridge_lock.

Fixes: 17805a15d175 ("drm/bridge: add list of removed refcounted bridges")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260723015004.1F5711F000E9@smtp.kernel.org/
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/drm_bridge.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

sashiko-bot@kernel.org Sept. 1, 2026, 7:17 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] drm_bridge_remove() destroys mutexes while the bridge is still refcounted, risking a use-after-destroy.
--

commit 7791e5cbe69e4f164e69cc2e98832df9a6224dd4
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/bridge: Fix unlocked list_del in drm_bridge_add()

When re-adding a bridge that was previously removed, drm_bridge_add() drops it
from bridge_lingering_list without holding bridge_lock. This commit ensures both
the list_empty() test and list_del_init() are performed under bridge_lock to
prevent list corruption.

Closes: https://lore.kernel.org/all/20260723015004.1F5711F000E9@smtp.kernel.org/

> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index afaae272347c0..2c457ad74f3b3 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -454,8 +454,10 @@ void drm_bridge_add(struct drm_bridge *bridge)
>  	 * in bridge_lingering_list. Remove it or bridge_lingering_list will be
>  	 * corrupted when adding this bridge to bridge_list below.
>  	 */
> +	mutex_lock(&bridge_lock);
>  	if (!list_empty(&bridge->list))
>  		list_del_init(&bridge->list);
> +	mutex_unlock(&bridge_lock);
>  
>  	mutex_init(&bridge->hpd_state_mutex);
>  	mutex_init(&bridge->hpd_mutex);
[Severity: High]
This is a pre-existing issue, but since we are modifying the bridge locking
here, is it safe for drm_bridge_remove() to unconditionally destroy these
mutexes while the bridge might still be refcounted?

When a bridge driver is unbound, drm_bridge_remove() calls:

	mutex_destroy(&bridge->hpd_mutex);
	mutex_destroy(&bridge->hpd_state_mutex);

If the DRM device still holds a reference to the bridge via drm_bridge_get(),
and subsequently attempts to call functions like drm_bridge_hpd_enable(), it
will lock a destroyed mutex:

	mutex_lock(&bridge->hpd_state_mutex);
	mutex_lock(&bridge->hpd_mutex);

Could this result in memory corruption or lockdep splats if the display
controller retains a reference after the bridge is removed?
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index afaae272347c..2c457ad74f3b 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -454,8 +454,10 @@  void drm_bridge_add(struct drm_bridge *bridge)
 	 * in bridge_lingering_list. Remove it or bridge_lingering_list will be
 	 * corrupted when adding this bridge to bridge_list below.
 	 */
+	mutex_lock(&bridge_lock);
 	if (!list_empty(&bridge->list))
 		list_del_init(&bridge->list);
+	mutex_unlock(&bridge_lock);
 
 	mutex_init(&bridge->hpd_state_mutex);
 	mutex_init(&bridge->hpd_mutex);