diff mbox series

[v11,13/74] drm/bridge: Fix NULL deref in drm_bridge_add() for legacy bridges

Message ID 20260901-dw-hdmi-qp-scramb-v11-13-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 
Legacy bridge drivers that embed struct drm_bridge in a zero-initialized
allocation, rather than obtaining it from devm_drm_bridge_alloc(), never
run INIT_LIST_HEAD() on bridge->list, leaving next and prev NULL.
list_empty() compares next against &bridge->list, so it reports such a
list head as non-empty.  list_del_init() therefore runs and dereferences
the NULL pointers, panicking the kernel during probe.  Only builds with
CONFIG_DEBUG_LIST survive, with a list corruption report.

This affects rk3066_hdmi and the i.MX8 LDB bridges, which still embed a
bridge in a devm_kzalloc()'d struct.

Initialize the list head when it is found to be NULL, so that the bridge
ends up with a valid empty list head and list_del_init() is only reached
for bridges that can actually be linked.

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

Comments

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

Pre-existing issues:
- [High] Legacy bridge drivers allocating a zero-initialized struct drm_bridge fail to initialize the `refcount` field. When `drm_bridge_add()` calls `drm_bridge_get()`, `kref_get()` increments the 0-valued refcount, triggering a `WARN_ONCE` and permanently setting the refcount to `REFCOUNT_SATURATED`. On driver teardown, `drm_bridge_remove()` moves the bridge to `bridge_lingering_list` and calls `drm_bridge_put()`. Due to the saturated refcount, it never drops to 0, preventing `__drm_bridge_free()` from unlinking the bridge from `bridge_lingering_list`. The backing memory is then freed by `devres`, leaving a dangling pointer in the global `bridge_lingering_list` that causes a use-after-free kernel panic if traversed (e.g., via the `/sys/kernel/debug/dri/bridges` debugfs file or further bridge removals).
--

commit 752e06bb4d7b734ae3e24f68e339df3e0062a55d
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

    drm/bridge: Fix NULL deref in drm_bridge_add() for legacy bridges

    This commit fixes a NULL pointer dereference in drm_bridge_add() for legacy
    bridges that embed struct drm_bridge in a zero-initialized allocation. It
    adds a check to initialize the list head if list.next is NULL.
    
    Closes: https://lore.kernel.org/all/20260731175016.C5D591F00AC4@smtp.kernel.org/

> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 2c457ad74f3b3..a8b6df5c13eaf 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -453,9 +453,16 @@ void drm_bridge_add(struct drm_bridge *bridge)
>  	 * If the bridge was previously added and then removed, it is now
>  	 * in bridge_lingering_list. Remove it or bridge_lingering_list will be
>  	 * corrupted when adding this bridge to bridge_list below.
> +	 *
> +	 * Legacy drivers that allocate the bridge with kzalloc() rather than
> +	 * devm_drm_bridge_alloc() leave list.next NULL. Such a bridge cannot
> +	 * be on any list, and list_del_init() would dereference NULL, so
> +	 * initialize the list head first.

[Severity: High]
This isn't a bug introduced by this patch, but since legacy bridge drivers
allocate a zero-initialized struct drm_bridge, doesn't this also mean the
refcount field is uninitialized?

When drm_bridge_add() is called, it executes drm_bridge_get() on the bridge
which increments the 0-valued refcount. This triggers a WARN_ONCE and
permanently sets the refcount to REFCOUNT_SATURATED.

Later, during driver teardown, the bridge is moved to the lingering list:

drivers/gpu/drm/drm_bridge.c:drm_bridge_remove() {
	mutex_lock(&bridge_lock);
	list_move_tail(&bridge->list, &bridge_lingering_list);
	mutex_unlock(&bridge_lock);
    ...
	drm_bridge_put(bridge);
}

Since the refcount is saturated, drm_bridge_put() fails to drop it to 0. The
bridge is stranded in the lingering list, preventing __drm_bridge_free() from
unlinking it.

Once devres frees the structure, the dangling pointer in the global
bridge_lingering_list can cause a use-after-free kernel panic if traversed
(e.g., via the /sys/kernel/debug/dri/bridges debugfs file or further bridge
removals).

Could the refcount initialization also be handled for kzalloc'd bridges to
prevent this leak and use-after-free?

>  	 */
>  	mutex_lock(&bridge_lock);
> -	if (!list_empty(&bridge->list))
> +	if (!bridge->list.next)
> +		INIT_LIST_HEAD(&bridge->list);
> +	else if (!list_empty(&bridge->list))
>  		list_del_init(&bridge->list);
>  	mutex_unlock(&bridge_lock);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 2c457ad74f3b..a8b6df5c13ea 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -453,9 +453,16 @@  void drm_bridge_add(struct drm_bridge *bridge)
 	 * If the bridge was previously added and then removed, it is now
 	 * in bridge_lingering_list. Remove it or bridge_lingering_list will be
 	 * corrupted when adding this bridge to bridge_list below.
+	 *
+	 * Legacy drivers that allocate the bridge with kzalloc() rather than
+	 * devm_drm_bridge_alloc() leave list.next NULL. Such a bridge cannot
+	 * be on any list, and list_del_init() would dereference NULL, so
+	 * initialize the list head first.
 	 */
 	mutex_lock(&bridge_lock);
-	if (!list_empty(&bridge->list))
+	if (!bridge->list.next)
+		INIT_LIST_HEAD(&bridge->list);
+	else if (!list_empty(&bridge->list))
 		list_del_init(&bridge->list);
 	mutex_unlock(&bridge_lock);