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
