[PATCH v2 30/34] drm/bridge: imx8qxp-pixel-combiner: convert to devm_drm_bridge_alloc() API

[Liu Ying]

On 05/07/2025, Luca Ceresoli wrote:

[...]

>> After looking into this patch and patch 31(though I've already provided my A-b)
>> more closely, I think the imx8qxp_pc and imx8{qm,qxp}_ldb main structures
>> should have the same life time with the embedded DRM bridges, because for
>> example the clk_apb clock in struct imx8qxp_pc would be accessed by the
>> imx8qxp_pc_bridge_mode_set DRM bridge callback.  But, IIUC, your patches extend
>> the life time for the embedded channel/bridge structures only, but not for the
>> main structures.  What do you think ?
> 
> I see you concern, but I'm sure the change I'm introducing is not
> creating the problem you are concerned about.
> 
> The key aspect is that my patch is merely changing the lifetime of the
> _allocation_ of the drm_bridge, not its usage. On drm_bridge_remove()
> the bridge is removed from its encoder chain and it is completely not
> reachable, both before and after my patch. With my patch it is not

drm_bridge_remove() only removes a bridge from the global bridge_list defined
in drm_bridge.c.  drm_bridge_detach() is the one which removes a bridge from
it's encoder chain.  It looks like you wrongly thought drm_bridge_remove()
is drm_bridge_detach().  So, even if drm_bridge_remove() is called, the removed
bridge could still be in it's encoder chain, hence an atomic commit could still
access the allocated bridge(with lifetime extended) and the clock_apb clock
for example in struct imx8qxp_pc could also be accessed.  That's why I think
the main structures should have the same lifetime with the allocated bridge.

> freed immediately, but it's just a piece of "wasted" memory that is
> still allocated until elsewhere in the kernel there are pointers to it,
> to avoid use-after-free.
> 
> With this explanation, do you think my patch is correct (after fixing
> the bug we already discussed of course)?
> 
> Best regards,
> Luca
> 

-- 
Regards,
Liu Ying