Re: [PATCH v11 13/74] drm/bridge: Fix NULL deref in drm_bridge_add() for legacy bridges
From: Cristian Ciocaltea <hidden>
Date: 2026-09-22 21:24:41
Also in:
dri-devel, linux-arm-kernel, linux-rockchip, linux-sunxi, lkml
Hi Luca, On 9/21/26 10:05 AM, Luca Ceresoli wrote:
On Tue Sep 8, 2026 at 11:24 AM CEST, Maxime Ripard wrote:quoted
On Tue, Sep 01, 2026 at 09:50:37PM +0300, Cristian Ciocaltea wrote:quoted
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/ (local) Signed-off-by: Cristian Ciocaltea <redacted> --- drivers/gpu/drm/drm_bridge.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)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."legacy" bridges are ambigous and was also used for non-atomic bridges. That being said, it's now *required* that bridges use devm_drm_bridge_alloc() instead of kzalloc, and we warn in drm_bridge_add if it hasn't. So I don't think we should merge this patch.I agree. I theory all drivers are be already using devm_drm_bridge_alloc() by now. In practice rk3066_hdmi is actually still using kzalloc, it should be fixed to use devm_drm_bridge_alloc(). Are you OK with doing it in v12?
Yes, I'll send this a separate series, together with the other bridge fixes.
By "i.MX8 LDB" I guess you mean fsl-ldb.c, which was converted in
9c399719cfb9 ("drm: convert many bridge drivers from devm_kzalloc() to
devm_drm_bridge_alloc() API").I was actually referring to imx8q*-ldb.c, but it turns out those were converted as well - I incorrectly flagged them because the probe functions still contain imx8qm_ldb = devm_kzalloc(dev, sizeof(*imx8qm_ldb), GFP_KERNEL); I've just realized the struct drm_bridge is not embedded in struct imx8qm_ldb,but it's part of struct ldb_channel. Sorry for the confusion. Thanks for the reviews & tests! Regards, Cristian