[PATCH v13 04/18] drm: exynos: dsi: Switch to DSI panel or bridge find helper

[Jagan Teki]

On Tue, Feb 28, 2023 at 8:05 PM Maxime Ripard <maxime at cerno.tech> wrote:
>
> On Tue, Feb 28, 2023 at 06:04:39PM +0530, Jagan Teki wrote:
> > On Tue, Feb 28, 2023 at 5:40 PM Maxime Ripard <maxime at cerno.tech> wrote:
> > >
> > > On Mon, Feb 27, 2023 at 08:41:22PM +0100, Marek Vasut wrote:
> > > > > If we go ahead with no need for DRM-managed helper at the moment, then
> > > > > find the panel hook in host.attach and then drop 2/18.
> > > >
> > > > The panel lookup must happen in .bind/.probe for exynos/imx respectively ,
> > > > that's really all that is required here. Then you can drop 1,2,3/18 and get
> > > > this series applied (I hope) .
> > > >
> > > > Can you implement just this one change ?
> > > >
> > > > There is no need to use drmm_* helper for now, that can be improved later if
> > > > possible.
> > >
> > > Yeah... The drmm helper isn't needed per se, but not using it will
> > > create a use-after-free pattern that is very easy to miss.
> > >
> > > I'd really prefer not to add a new helper that favors an unsafe pattern,
> > > but the driver seems to have a whole bunch of them anyway so it's not
> > > really a big deal.
> > >
> > > Which also raises another question: if it's code that is only really
> > > relevant in the context of that driver, why are you creating a helper
> > > for it in the first place?
> >
> > I can answer this question as I did add these helpers.
> >
> > 1. DSI-specific helper added since it is a good candidate for
> > common/helper code, based on the comments in V9 by Marek. V
> > https://patchwork.kernel.org/project/dri-devel/patch/20221209152343.180139-8-jagan@amarulasolutions.com/
> >
> > So, I have added this to the common drm_of code in V10.
> > https://patchwork.kernel.org/project/dri-devel/patch/20221214125907.376148-2-jagan@amarulasolutions.com/
> >
> > 2. DRM-managed discussion was commented on in V11 by you, so from
> > where all discussion moved.
> > https://patchwork.kernel.org/project/dri-devel/patch/20230123151212.269082-3-jagan@amarulasolutions.com/
> >
> > 1) helper wouldn't be an unsafe helper as it can reuse many DSI
> > drivers but 2) helper might be an unsafe helper at the moment.
>
> You're wrong. The first one is unsafe, for the same reason than the devm
> one you did is unsafe: the assumption everyone will get (and the one you
> implemented in your driver) is that the bridge reference will be put
> back at remove time.
>
> The DRM/KMS structures however are free'd only when the last user closes
> the file descriptor of the KMS device, so your driver functions will get
> called after remove has been called.
>
> If you are using the panel or bridge in any of your KMS hooks
> implementations, this is now a use-after-free.
>
> The drmm variant is safe though, because drmm actions run not when the
> device is removed but when the DRM device is last closed.

Okay. So, even if we manually use drm_of_dsi_find_panel_or_bridge in
mipi_dsi_host_ops.attach and drm_panel_bridge_remove() in
mipi_dsi_host_ops.detach KMS doesn't aware of it and is still unsafe.
- am I correct?

Jagan.