Re: [PATCH] libv4l: add NULL pointer check

["Németh Márton" <nm127@freemail.hu>]

Laurent Pinchart wrote:
> On Sunday 30 August 2009 10:56:16 Németh Márton wrote:
>> From: Márton Németh <nm127@freemail.hu>
>>
>> Add NULL pointer check before the pointers are dereferenced.
> 
> Applications are not supposed to pass NULL pointers to those functions. It 
> would be an API violation. Instead of silently failing a segfault is better.

Actually we cannot speak about API violation because the behaviour when
passing NULL pointer as ioctl() argument is not defined as of V4L2 API
revision 0.29 available from http://linuxtv.org/hg/v4l-dvb/ . The current
implemention in Linux in kernel space is to return -EACCESS.

I don't really agree with the segfault behaviour, because:

 - currently there is a different behaviour when just using the V4L2
   interface and using the libv4l2 0.6.0. When using the kernel interface
   it is an error in kernelspace if a NULL pointer is dereferenced, thus
   kernel will return -EACCESS. When the libv4l2 0.6.0 is used then the
   behaviour changes: currently there is a segfault instead of getting
   a return value -1 and errno=EACCESS.

 - the segfault normally results that the whole calling process is
   killed. If there is a complex software like a browser, it is not
   very user friendly that the whole software crashes just because an
   implementation error in the V4L2 handling code.

 - currently a lot of V4L2 API ioctls() return -EINVAL or -EFAULT when
   passing NULL pointer as a parameter depending on whether the given
   ioctl is not supported at all or it is supported but there is a problem
   with the passed pointer, respectively. The use case for this would be
   that an application could easily scan for the supported and not supported
   V4L2 ioctls.

 - dereferencing a NULL pointer is not always result segfault, see [1] and
   [2]. So dereferencing a NULL pointer can be treated also as a security
   risk.

 - the patch I sent is only checking the pointer just before it is
   dereferenced. When the libv4l just passes the pointer value to the
   ioctl() then there is no additional check: this situation will be
   handled in kernel space.

These are my arguments. I am open to listen to your arguments.

I think that the final solution could be that the V4L2 API specification
defines what shall happen when NULL pointer is passed as an ioctl() argument.

References:
[1] Jonathan Corbet: Fun with NULL pointers, part 1 (July 20, 2009)
    http://lwn.net/Articles/342330/

[2] Jonathan Corbet: Fun with NULL pointers, part 2
    http://lwn.net/Articles/342420/

Regards,

	Márton Németh