diff mbox series

[v10,12/69] drm/bridge: Fix unlocked list_del in drm_bridge_add()

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

Commit Message

Cristian Ciocaltea July 31, 2026, 4:19 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 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

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

Pre-existing issues:
- [High] Legacy zero-initialized bridges not allocated via devm_drm_bridge_alloc() will cause a kernel panic due to an uninitialized list_head and trigger refcount warnings.
--

--- Patch [12]: [PATCH v10 12/69] drm/bridge: Fix unlocked list_del in drm_bridge_add() ---
commit e0f10da4c05d8a75c537ce94edb147bda1eb2e78
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/bridge: Fix unlocked list_del in drm_bridge_add()

This commit addresses a list corruption issue in drm_bridge_add() by wrapping 
the bridge_lingering_list removal sequence with the bridge_lock mutex. It 
prevents concurrent modifications from drm_bridge_remove() and debugfs walks.

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..8b8f71c3ccff9 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c

[ ... ]

> @@ -454,8 +455,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.
>  	 */
> -	if (!list_empty(&bridge->list))
> -		list_del_init(&bridge->list);
> +	scoped_guard(mutex, &bridge_lock) {
> +		if (!list_empty(&bridge->list))
> +			list_del_init(&bridge->list);
> +	}

[Severity: High]
This is a pre-existing issue, but will this cause a kernel panic for legacy 
bridge drivers that allocate struct drm_bridge using kzalloc() instead of 
devm_drm_bridge_alloc()?

For a zero-initialized bridge struct, the list_empty() check evaluates to 
false because the uninitialized list_head pointers are NULL, and NULL is not 
equal to &bridge->list. 

This means list_del_init() will execute and unconditionally dereference the 
NULL prev and next pointers, crashing the kernel during device probe.

Is it possible to check if the list_head is actually initialized before 
testing if it is empty, or handle legacy zero-initialized bridges safely 
to avoid breaking the probe path on affected platforms?

>  
>  	mutex_init(&bridge->hpd_state_mutex);
>  	mutex_init(&bridge->hpd_mutex);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index afaae272347c..8b8f71c3ccff 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -21,6 +21,7 @@ 
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include <linux/cleanup.h>
 #include <linux/debugfs.h>
 #include <linux/err.h>
 #include <linux/export.h>
@@ -454,8 +455,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.
 	 */
-	if (!list_empty(&bridge->list))
-		list_del_init(&bridge->list);
+	scoped_guard(mutex, &bridge_lock) {
+		if (!list_empty(&bridge->list))
+			list_del_init(&bridge->list);
+	}
 
 	mutex_init(&bridge->hpd_state_mutex);
 	mutex_init(&bridge->hpd_mutex);