On Mon, Aug 22, 2016 at 05:02:31PM +0300, Sakari Ailus wrote:
> Hi Hans,
>
> On Mon, Aug 22, 2016 at 02:40:39PM +0200, Hans Verkuil wrote:
> > On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> > > devm functions are fine for managing resources that are directly related
> > > to the device at hand and that have no other dependencies. However, a
> > > process holding a file handle to a device created by a driver for a device
> > > may result in the file handle left behind after the device is long gone.
> > > This will result in accessing released (and potentially reallocated)
> > > memory.
> > >
> > > Instead, rely on the media device which will stick around until all users
> > > are gone.
> > >
> > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/media/platform/omap3isp/isp.c | 38 ++++++++++++++++++++-------
> > > drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
> > > drivers/media/platform/omap3isp/isph3a_aewb.c | 20 +++++++++-----
> > > drivers/media/platform/omap3isp/isph3a_af.c | 20 +++++++++-----
> > > drivers/media/platform/omap3isp/isphist.c | 5 ++--
> > > drivers/media/platform/omap3isp/ispstat.c | 2 ++
> > > 6 files changed, 63 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> > > index 217d4da..3488ed3 100644
> > > --- a/drivers/media/platform/omap3isp/isp.c
> > > +++ b/drivers/media/platform/omap3isp/isp.c
> > > @@ -1370,7 +1370,7 @@ static int isp_get_clocks(struct isp_device *isp)
> > > unsigned int i;
> > >
> > > for (i = 0; i < ARRAY_SIZE(isp_clocks); ++i) {
> > > - clk = devm_clk_get(isp->dev, isp_clocks[i]);
> >
> > I wonder, would it be possible to use the media device itself for these devm_
> > functions? Since the media device is the last one to be released...
>
> Do you happen to mean... struct media_device->devnode.dev?
>
> Interesting idea, I can't see why not. That'd actually make the required
> driver changes to fix the drivers quite a bit easier to make. And we could
> still use devm_() functions.
That might have been a little bit too fast.
In fact, device_release() in drivers/base/core.c does release resources
bound to device (devm) *before* calling the driver's release callback.
This means that the driver's device release callback may not access
resources allocated with devm functions. Considering that devm_() functions
have been broken and fixed before [1], could this be more than a bug?
The comments there seem off topic mostly.
[1] a525a3ddeaca69f405d98442ab3c0746e53168dc
--
Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
