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 <[email protected]> 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/[email protected]/ > 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/[email protected]?part=13
