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 <[email protected]> >> >> 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-667abf6d4...@bootlin.com/ Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
