Laurent Pinchart wrote: > On Sunday 30 August 2009 10:56:16 Németh Márton wrote: >> From: Márton Németh <nm127@xxxxxxxxxxx> >> >> 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 -- 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
