On Wed, Jan 20, 2021 at 6:12 PM Paul Cercueil <paul@xxxxxxxxxxxxxxx> wrote:
>
>
>
> Le mer. 20 janv. 2021 à 17:03, Daniel Vetter <daniel@xxxxxxxx> a
> écrit :
> > On Wed, Jan 20, 2021 at 1:35 PM Paul Cercueil <paul@xxxxxxxxxxxxxxx>
> > wrote:
> >>
> >> If we don't call drm_connector_cleanup() manually in
> >> panel_bridge_detach(), the connector will be cleaned up with the
> >> other
> >> DRM objects in the call to drm_mode_config_cleanup(). However,
> >> since our
> >> drm_connector is devm-allocated, by the time
> >> drm_mode_config_cleanup()
> >> will be called, our connector will be long gone. Therefore, the
> >> connector must be cleaned up when the bridge is detached to avoid
> >> use-after-free conditions.
> >
> > For -fixes this sounds ok, but for -next I think switching to drmm_
> > would be much better.
>
> The API would need to change to have access to the drm_device struct,
> though. That would be quite a big patch, there are a few dozens source
> files that use this API already.
Hm right pure drmm_ doesn't work for panel or bridge since it's
usually a separate driver. But devm_ also doesn't work. I think what
we need here is two-stage: first kmalloc the panel (or bridge, it's
really the same) in the panel/bridge driver load. Then when we bind it
to the drm_device we can tie it into the managed resources with
drmm_add_action_or_reset. Passing the drm_device to the point where we
allocate the panel/bridge doesn't work for these.
I think minimally we need a FIXME here and ack from Laurent on how
this should be solved at least, since panel bridge is used rather
widely.
-Daniel
>
> Cheers,
> -Paul
>
> >
> >> v2: Cleanup connector only if it was created
> >>
> >> Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper
> >> from the lvds-encoder bridge.")
> >> Cc: <stable@xxxxxxxxxxxxxxx> # 4.12+
> >> Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
> >> Cc: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> >> Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx>
> >> Cc: Jonas Karlman <jonas@xxxxxxxxx>
> >> Cc: Jernej Skrabec <jernej.skrabec@xxxxxxxx>
> >> Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/bridge/panel.c | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/panel.c
> >> b/drivers/gpu/drm/bridge/panel.c
> >> index 0ddc37551194..df86b0ee0549 100644
> >> --- a/drivers/gpu/drm/bridge/panel.c
> >> +++ b/drivers/gpu/drm/bridge/panel.c
> >> @@ -87,6 +87,12 @@ static int panel_bridge_attach(struct drm_bridge
> >> *bridge,
> >>
> >> static void panel_bridge_detach(struct drm_bridge *bridge)
> >> {
> >> + struct panel_bridge *panel_bridge =
> >> drm_bridge_to_panel_bridge(bridge);
> >> + struct drm_connector *connector = &panel_bridge->connector;
> >> +
> >> + /* Cleanup the connector if we know it was initialized */
> >> + if (!!panel_bridge->connector.dev)
> >> + drm_connector_cleanup(connector);
> >> }
> >>
> >> static void panel_bridge_pre_enable(struct drm_bridge *bridge)
> >> --
> >> 2.29.2
> >>
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
