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

[sashiko-bot@kernel.org]

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);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-dw-hdmi-qp-scramb-v11-0-bc12954a0688@collabora.com?part=13