[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <DDKQGM8BPW8T.3PTC93J4GHUR5@bootlin.com>
Date: Fri, 17 Oct 2025 18:20:59 +0200
From: "Luca Ceresoli" <luca.ceresoli@...tlin.com>
To: "Luca Ceresoli" <luca.ceresoli@...tlin.com>, "Marek Szyprowski"
<m.szyprowski@...sung.com>, "Maarten Lankhorst"
<maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
"Thomas Zimmermann" <tzimmermann@...e.de>, "David Airlie"
<airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>, "Andrzej Hajda"
<andrzej.hajda@...el.com>, "Neil Armstrong" <neil.armstrong@...aro.org>,
"Robert Foss" <rfoss@...nel.org>, "Laurent Pinchart"
<Laurent.pinchart@...asonboard.com>, "Jonas Karlman" <jonas@...boo.se>,
"Jernej Skrabec" <jernej.skrabec@...il.com>, "Dmitry Baryshkov"
<dmitry.baryshkov@....qualcomm.com>
Cc: "Hui Pu" <Hui.Pu@...ealthcare.com>, "Thomas Petazzoni"
<thomas.petazzoni@...tlin.com>, <dri-devel@...ts.freedesktop.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] drm/display: bridge_connector: get/put the stored
bridges
Hello,
On Wed Oct 15, 2025 at 10:08 PM CEST, Luca Ceresoli wrote:
> Hello Marek,
>
> On Wed Oct 15, 2025 at 10:22 AM CEST, Marek Szyprowski wrote:
>> Hi Luca,
>>
>> On 26.09.2025 16:59, Luca Ceresoli wrote:
>>> drm_bridge_connector_init() takes eight pointers to various bridges, some
>>> of which can be identical, and stores them in pointers inside struct
>>> drm_bridge_connector. Get a reference to each of the taken bridges and put
>>> it on cleanup.
>>>
>>> This is tricky because the pointers are currently stored directly in the
>>> drm_bridge_connector in the loop, but there is no nice and clean way to put
>>> those pointers on error return paths. To overcome this, store all pointers
>>> in temporary local variables with a cleanup action, and only on success
>>> copy them into struct drm_bridge_connector (getting another ref while
>>> copying).
>>>
>>> Additionally four of these pointers (edid, hpd, detect and modes) can be
>>> written in multiple loop iterations, in order to eventually store the last
>>> matching bridge. However, when one of those pointers is overwritten, we
>>> need to put the reference that we got during the previous assignment. Add a
>>> drm_bridge_put() before writing them to handle this.
>>>
>>> Finally, there is also a function-local panel_bridge pointer taken inside
>>> the loop and used after the loop. Use a cleanup action as well to ensure it
>>> is put on return.
>>>
>>> Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
>>
>> This patch landed recently in linux-next as commit 2be300f9a0b6
>> ("drm/display: bridge_connector: get/put the stored bridges"). In my
>> tests I found that it causes the following NULL pointer dereference on
>> DragonBoard410c (arch/arm64/boot/dts/qcom/apq8016-sbc.dts):
...
> Thanks for testing and reporting.
>
> I'm afraid I have no hardware where the same bug can be reproduced, but by
> code inspection the root cause is clear given the call chain:
>
> drm_bridge_connector_init() [1]
> -> drmm_connector_hdmi_cec_register() [2]
> -> funcs->init() = drm_bridge_connector_hdmi_cec_init() [3]
>
> [1] used to set bridge_connector->bridge_hdmi_cec before calling [2], now
> it does it afterwards. But [3] expects it to be set already.
>
> I have overlooked this when writing the patch. My apologies.
...
> I'm looking at how to properly fix this bug ASAP.
Here it is:
https://lore.kernel.org/lkml/20251017-drm-bridge-alloc-getput-bridge-connector-fix-hdmi_cec-v2-0-667abf6d47c0@bootlin.com/
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists