Re: Fw: zoran drivers: absense of locking?

["Ronald S. Bultje" <rbultje@ronald.bitfreak.net>]

Hi guys,

I maintain the drivers. So, let's see:

On Fri, 2005-10-28 at 21:45 -0400, Michael Krufky wrote:
> >>From: Alexey Dobriyan <adobriyan@gmail.com>
> >>I've tried to read random part of a tree and now scratching my head
> >>with a question:
> >>
> >>	what protects the number and a list of registered codecs in
> >>	zoran drivers?
> >>
> >>Example: drivers/media/video/zr36050.c:
> >>
> >>	/* amount of chips attached via this driver */
> >>	static int zr36050_codecs = 0;
> >>
> >>Decremented in zr36050_unset().
> >>Checked for maximum value, used and incremented in zr36050_setup().
> >>
> >>[Assigment to 0 in zr36050_init_module is not needed. dprintk() in
> >>zr36050_cleanup_module() should be converted to BUG_ON, so I'll ignore
> >>them.]
> >>
> >>	zr36050_codecs
> >>		zr36050_unset()	= struct videocodec::unset
> >>		zr36050_setup()	= struct videocodec::setup
> >>
> >>The only place where ->unset and ->setup methods are called is
> >>drivers/media/video/videocodec.c:
> >>
> >>	zr36050_codecs
> >>		zr36050_unset()
> >>			videocodec_detach()
> >>		zr36050_setup()
> >>			videocodec_attach()
> >>
> >>Both videocodec functions are exported.
> >>
> >>No spinlocks or semaphores in sight.
> >>
> >>Does anybody know what protects the list of registered codecs in zoran
> >>drivers?

So, you're theoretically right, this is indeed a theory problem. Now, in
practice it isn't. Why? Because this isn't an actually exported
documented interface. This means two things: userspace cannot access it
(so no security issue), and other drivers than the ones I maintain won't
use it (so locking issues are limited to my own driver set).

It's an internal interface to the zoran driver (zoran_*.c / zr36067.ko).
The zoran driver uses publically exported interfaces (v4l/v4l2), and
those can be accessed from userspace. The zoran driver uses proper
locking everywhere and ensures that nothing goes wrong (or, well, so I
hope). So since the zoran driver is the only driver accessing zr36050.ko
(through videocodec.ko) and uses proper locking itself already, all's
fine in practice.

If other projects intend to use the videocodec interface, I'll consider
adding locking everywhere. But for now, it's fine, I guess.

Cheers,
Ronald