Re: Fw: zoran drivers: absense of locking?

Re: Fw: zoran drivers: absense of locking?

[Michael Krufky]

Johannes Stezenbach wrote:

>On Fri, Oct 28, 2005 Andrew Morton wrote:
>  
>
>>Alexey, please don't assume that everyone reads lkml.
>>    
>>
>I Cc: the v4l people, maybe they can answer the question.
>
>Johannes
>
>  
>
It's not in our tree... This is what I can see in the MAINTAINERS file:

ZR36067 VIDEO FOR LINUX DRIVER
P:      Ronald Bultje
M:      rbultje@ronald.bitfreak.net
L:      mjpeg-users@lists.sourceforge.net
W:      http://mjpeg.sourceforge.net/driver-zoran/
S:      Maintained

ZR36120 VIDEO FOR LINUX DRIVER
P:      Pauline Middelink
M:      middelin@polyware.nl
W:      http://www.polyware.nl/~middelin/En/hobbies.html
W:      http://www.polyware.nl/~middelin/hobbies.html
S:      Maintained

I can only guess that this falls under ZR36067 ??  (cc's added)

>>Begin forwarded message:
>>
>>Date: Sat, 29 Oct 2005 01:16:47 +0400
>>From: Alexey Dobriyan <adobriyan@gmail.com>
>>To: linux-kernel@vger.kernel.org
>>Subject: zoran drivers: absense of locking?
>>
>>
>>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?
>>
>>-
>>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>Please read the FAQ at  http://www.tux.org/lkml/
>>

Re: Fw: zoran drivers: absense of locking?

[Ronald S. Bultje]

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

Re: Fw: zoran drivers: absense of locking?

[Pauline Middelink]

[-- Attachment #1: Type: text/plain, Size: 881 bytes --]

On Fri, 28 Oct 2005 around 21:45:04 -0400, Michael Krufky wrote:

> ZR36067 VIDEO FOR LINUX DRIVER
> P:      Ronald Bultje
> M:      rbultje@ronald.bitfreak.net
> L:      mjpeg-users@lists.sourceforge.net
> W:      http://mjpeg.sourceforge.net/driver-zoran/
> S:      Maintained
> 
> ZR36120 VIDEO FOR LINUX DRIVER
> P:      Pauline Middelink
> M:      middelin@polyware.nl
> W:      http://www.polyware.nl/~middelin/En/hobbies.html
> W:      http://www.polyware.nl/~middelin/hobbies.html
> S:      Maintained

> I can only guess that this falls under ZR36067 ??  (cc's added)

Totally different beasts... /me only does zr36120

[locking part snipped]

    Met vriendelijke groet,
        Pauline Middelink
-- 
GPG Key fingerprint = 2D5B 87A7 DDA6 0378 5DEA  BD3B 9A50 B416 E2D0 C3C2
For more details look at my website http://www.polyware.nl/~middelink

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2276 bytes --]