Re: [PATCH v2] drm/display: bridge_connector: get/put the stored bridges

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

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@bootlin.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