Re: [PATCH v11 2/3] drm/bridge: imx: Add i.MX93 parallel display format configuration support

[Liu Ying <victor.liu@nxp.com>]

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-b9ba3d844732@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