linux-kernel - Re: [RFC 07/13] v4l2: device_register_subdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170214221130.GP16975@valkosipuli.retiisi.org.uk>
Date:   Wed, 15 Feb 2017 00:11:31 +0200
From:   Sakari Ailus <sakari.ailus@....fi>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     Pavel Machek <pavel@....cz>, sre@...nel.org, pali.rohar@...il.com,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        mchehab@...nel.org, ivo.g.dimitrov.75@...il.com
Subject: Re: [RFC 07/13] v4l2: device_register_subdev_nodes: allow calling
 multiple times

Hi Laurent,

On Wed, Feb 15, 2017 at 12:06:17AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Wednesday 15 Feb 2017 00:02:57 Sakari Ailus wrote:
> > On Tue, Feb 14, 2017 at 02:40:00PM +0100, Pavel Machek wrote:
> > > From: Sebastian Reichel <sre@...nel.org>
> > > 
> > > Without this, exposure / gain controls do not work in the camera
> > > application.:
> > :-)
> > :
> > > Signed-off-by: Sebastian Reichel <sre@...nel.org>
> > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@...il.com>
> > > Signed-off-by: Pavel Machek <pavel@....cz>
> > > ---
> > > 
> > >  drivers/media/v4l2-core/v4l2-device.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-device.c
> > > b/drivers/media/v4l2-core/v4l2-device.c index f364cc1..b3afbe8 100644
> > > --- a/drivers/media/v4l2-core/v4l2-device.c
> > > +++ b/drivers/media/v4l2-core/v4l2-device.c
> > > @@ -235,6 +235,9 @@ int v4l2_device_register_subdev_nodes(struct
> > > v4l2_device *v4l2_dev)
> > >  		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
> > >  			continue;
> > > 
> > > +		if(sd->devnode)
> > > +			continue;
> > 
> > This has been recognised as a problem but you're the first to submit a patch
> > I believe. Please add an appropriate description. :-)
> > 
> > s/if\(/if (/
> > 
> > I think this one should go in before the rest.
> 
> But how can this happen ? Is v4l2_device_register_subdev_nodes() called 
> multiple times ? Do we want to allow that ?

A driver could do this even accidentally. It's much better to do the right
thing than to start corrupting system memory sooner or later.

In the future we'll need to be able to dynamically register device nodes
after the registration of the original device nodes in a media device has
completed anyway. I don't think this would be the means for that though.

What happens here though is that both the video bus switch and isp notifiers
completing will call the function, thus ending up trying to register many of
the nodes twice. Philipp had a different approach to the problem ---
postponing the complete until add the sub-devices have been bound. The patch
is called "v4l2-async: allow subdevices to add further subdevices to the
notifier waiting list".

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@....fi	XMPP: sailus@...iisi.org.uk