Re: [PATCH v2 1/3] drm: bridge/panel: Cleanup connector on bridge detach

[Paul Cercueil]

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.

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