On Fri, Sep 26, 2025 at 04:59:40PM +0200, 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 series ensures the bridge-connector gets a reference to bridges when > storing a pointer to them, and releases them afterwards. > > This is part of the work towards removal of bridges from a still existing > DRM pipeline without use-after-free. The grand plan was discussed in [1]. > Here's the work breakdown (➜ marks the current series): > > 1. ➜ add refcounting to DRM bridges (struct drm_bridge) > (based on devm_drm_bridge_alloc() [0]) > A. ✔ add new alloc API and refcounting (v6.16) > B. ✔ convert all bridge drivers to new API (v6.17) > C. ✔ kunit tests (v6.17) > D. ✔ add get/put to drm_bridge_add/remove() + attach/detach() > and warn on old allocation pattern (v6.17) > E. … add get/put on drm_bridge accessors > 1. ✔ drm_bridge_chain_get_first_bridge() + add a cleanup action > (drm-misc-next) > 2. ✔ drm_bridge_get_prev_bridge() (drm-misc-next) > 3. ✔ drm_bridge_get_next_bridge() (drm-misc-next) > 4. ✔ drm_for_each_bridge_in_chain() (drm-misc-next) > 5. ➜ drm_bridge_connector_init > 6. protect encoder bridge chain with a mutex > 7. of_drm_find_bridge > 8. drm_of_find_panel_or_bridge, *_of_get_bridge > F. ➜ debugfs improvements > 1. ✔ add top-level 'bridges' file (v6.16) > 2. ✔ show refcount and list removed bridges (drm-misc-next) > 2. … handle gracefully atomic updates during bridge removal > 3. … DSI host-device driver interaction > 4. removing the need for the "always-disconnected" connector > 5. finish the hotplug bridge work, moving code to the core and potentially > removing the hotplug-bridge itself (this needs to be clarified as > points 1-3 are developed) > > This was tricky both because there is no central place in > drm_bridge_connector.c to put the references on disposal (handled by patch > 1) and because of the complex code flow of drm_bridge_connector_init() > (handled by patch 2). > --- > Changes in v2: > - Use drmm_add_action() instead of hacking the .destroy connector func > - Removed patch 1 (where the hacking the .destroy connector func was) > - Link to v1: > https://lore.kernel.org/r/20250925-drm-bridge-alloc-getput-bridge-connector-v1-0-f0736e1c7...@bootlin.com > --- > drivers/gpu/drm/display/drm_bridge_connector.c | 114 > +++++++++++++++++-------- > 1 file changed, 78 insertions(+), 36 deletions(-) >
Reviewed-by: Dmitry Baryshkov <[email protected]> -- With best wishes Dmitry
