On Wed, Mar 11, 2026 at 08:45:25PM +0100, Marco Felsch wrote: > Hi Liu, Luca,
Hi, > > sorry for the delayed response, I was at the EW26. Np at all, it's good to go out to meet people sometimes :) > > On 26-03-10, Luca Ceresoli wrote: >> Hi Liu, Marco, >> >> On Tue Mar 10, 2026 at 3:57 AM CET, Liu Ying wrote: >>> Hi Marco, Luca, >>> >>> On Tue, Mar 03, 2026 at 11:34:27AM +0100, Marco Felsch wrote: >>> >>> [...] >>> >>>> + next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0); >>>> + if (IS_ERR(next_bridge)) >>>> + return dev_err_probe(dev, PTR_ERR(next_bridge), >>>> + "failed to get next bridge\n"); >>>> + pdfc->dev = dev; >>>> + pdfc->bridge.of_node = dev->of_node; >>>> + pdfc->bridge.type = DRM_MODE_CONNECTOR_DPI; >>>> + pdfc->bridge.next_bridge = next_bridge; >>> >>> When I was reviewing another patch[1], I was aware of the necessity of >>> calling drm_bridge_get() for next_bridge to balance the next bridge's >>> refcount put from __drm_bridge_free() for this bridge. I'd be good if >>> Luca may confirm this is correct. Sorry for bringing this up late. >> >> Indeed you have a good point. > > At which stage did you faced this issue? During driver probe, because of > EPROBE_DEFER? I just want to balance the get/put for the next bridge's refcount by calling drm_bridge_get() for next_bridge, as I said above. There could be some way to trigger some particular Use-After-Free issues if you don't do that. I did take some time today to try to trigger UAF, but no luck, though I found a potential bug in encoder_bridges_show() and generated a fix[1](Cc'ed Marco) when I played with debugfs to read the refcounts. My idea to trigger UAF is to remove imx_lcdif/imx93_pdfc/simple_panel modules when doing pageflips. Dunno if EPROBE_DEFER may trigger UAF. [1] https://lore.kernel.org/all/20260312-drm-misc-next-2026-03-05-fix-encoder-bridges-refcount-v1-1-b9ba3d844...@nxp.com/ > > That's the reason for having the local next_bridge variable since I > faced with the same issue. In other words this driver is correct and > it's on purpose to not assign it directly. Albeit I could/should have > added a comment. What's the issue your faced? How would using next_bridge variable impact? Without knowing these info, I presume that you still need to call drm_bridge_get() for next_bridge, since it's kind of obvious if you take a look at __drm_bridge_free(). > >> After re-checking devm_drm_of_get_bridge(), as I wrote on the other thread >> you pointed to, you should call drm_bridge_get(): >> >> - pdfc->bridge.next_bridge = next_bridge; >> + pdfc->bridge.next_bridge = drm_bridge_get(next_bridge); >> >> Marco, you can keep my R-by if you resend with just this change. >> >> Sorry about the confusion here. >> >> As mention on the other thread, devm_drm_of_get_bridge() is unable to >> support bridge hotplug. So it should be deprecated, but as of now there is >> no alternative. > > Sorry I need a bit more context. What's the issue? How can I trigger the > issue? Why is bridge hotplug required at this stage? Why is only this > bridge affecte by the hotplug issue? Maybe, Luca can comment here. > > Regards, > Marco > > > >> >> Luca >> >> -- >> Luca Ceresoli, Bootlin >> Embedded Linux and Kernel engineering >> https://bootlin.com/ >> > -- Regards, Liu Ying
