[PATCH] videodev fixups / generic usercopy helper

Originally from Gerd...

I've just noticed that a hole left in the recent changes which should
allow the usb v4l drivers to unregister with open file handles.  The
drivers itself handle it just fine, but video_generic_ioctl() will barf
when called on unregistered devices.  Oops.

One way to fix this is to expect drivers call the helper function and
pass a pointer for the function doing the actual work, i.e. handle it
this way:

driver_ioctl(inode,file,cmd,userptr)
	-> video_usercopy(inode,file,cmd,userptr,func)
		copy_from_user(...)
		-> func(inode,file,cmd,kernelptr);
		copy_to_user(...)

Patch against 2.5.7-pre2 below. It updates videodev.[ch] and adapts
usbvideo.c to show how the driver changes will look like.

Note that this change makes the usercopy helper function a very generic
one, it probably could be used for other drivers to (as long as the API
has sane magic numbers based on _IO*(...) defines) as there is no
video4linux-related stuff in there any more.  So we might think of
renaming it an moving it to some more central place (fs/ioctl.c maybe).

diff --git a/drivers/media/video/videodev.c b/drivers/media/video/videodev.c
index d69f8378ca794e5350ffa3e0d8adf9e54e7644fd..834d86e45f49c9d1438e6b47755775b7038cc483 100644 (file)
--- a/drivers/media/video/videodev.c
+++ b/drivers/media/video/videodev.c
@@ -107,20 +107,18 @@ static int video_open(struct inode *inode, struct file *file)
 }
 
 /*
- * ioctl helper function -- handles userspace copying
+ * helper function -- handles userspace copying for ioctl arguments
  */
 int
-video_generic_ioctl(struct inode *inode, struct file *file,
-                   unsigned int cmd, unsigned long arg)
+video_usercopy(struct inode *inode, struct file *file,
+              unsigned int cmd, unsigned long arg,
+              int (*func)(struct inode *inode, struct file *file,
+                          unsigned int cmd, void *arg))
 {
-       struct  video_device *vfl = video_devdata(file);
        char    sbuf[128];
        void    *mbuf = NULL;
        void    *parg = NULL;
        int     err  = -EINVAL;
-       
-       if (vfl->kernel_ioctl == NULL)
-               return -EINVAL;
 
        /*  Copy arguments into temp kernel buffer  */
        switch (_IOC_DIR(cmd)) {
@@ -147,7 +145,7 @@ video_generic_ioctl(struct inode *inode, struct file *file,
        }
 
        /* call driver */
-       err = vfl->kernel_ioctl(inode, file, cmd, parg);
+       err = func(inode, file, cmd, parg);
        if (err == -ENOIOCTLCMD)
                err = -EINVAL;
        if (err < 0)
@@ -512,7 +510,7 @@ module_exit(videodev_exit)
 EXPORT_SYMBOL(video_register_device);
 EXPORT_SYMBOL(video_unregister_device);
 EXPORT_SYMBOL(video_devdata);
-EXPORT_SYMBOL(video_generic_ioctl);
+EXPORT_SYMBOL(video_usercopy);
 EXPORT_SYMBOL(video_exclusive_open);
 EXPORT_SYMBOL(video_exclusive_release);
 

diff --git a/drivers/usb/usbvideo.c b/drivers/usb/usbvideo.c
index b0d4eb5e0cdec49fdf92a9e7b48c0481f30d1c40..c04a67e6c45c5744b2fedb9cdab3bfaa29c695c8 100644 (file)
--- a/drivers/usb/usbvideo.c
+++ b/drivers/usb/usbvideo.c
@@ -1020,7 +1020,7 @@ static struct file_operations usbvideo_fops = {
        release:  usbvideo_v4l_close,
        read:     usbvideo_v4l_read,
        mmap:     usbvideo_v4l_mmap,
-       ioctl:    video_generic_ioctl,
+       ioctl:    usbvideo_v4l_ioctl,
        llseek:   no_llseek,
 };
 static struct video_device usbvideo_template = {
@@ -1028,7 +1028,6 @@ static struct video_device usbvideo_template = {
        type:         VID_TYPE_CAPTURE,
        hardware:     VID_HARDWARE_CPIA,
        fops:         &usbvideo_fops,
-       kernel_ioctl: usbvideo_v4l_ioctl,
 };
 
 uvd_t *usbvideo_AllocateDevice(usbvideo_t *cams)
@@ -1349,8 +1348,8 @@ int usbvideo_v4l_close(struct inode *inode, struct file *file)
  * History:
  * 22-Jan-2000 Corrected VIDIOCSPICT to reject unsupported settings.
  */
-int usbvideo_v4l_ioctl(struct inode *inode, struct file *file,
-                      unsigned int cmd, void *arg)
+static int usbvideo_v4l_do_ioctl(struct inode *inode, struct file *file,
+                                unsigned int cmd, void *arg)
 {
        uvd_t *uvd = file->private_data;
 
@@ -1555,6 +1554,12 @@ int usbvideo_v4l_ioctl(struct inode *inode, struct file *file,
        return 0;
 }
 
+int usbvideo_v4l_ioctl(struct inode *inode, struct file *file,
+                      unsigned int cmd, unsigned long arg)
+{
+       return video_usercopy(inode, file, cmd, arg, usbvideo_v4l_do_ioctl);
+}
+
 /*
  * usbvideo_v4l_read()
  *

diff --git a/drivers/usb/usbvideo.h b/drivers/usb/usbvideo.h
index 006fd267a42600bca2989095e2d500fdb1091c2b..211249c340db4d541f13eb81d163bfee639506ae 100644 (file)
--- a/drivers/usb/usbvideo.h
+++ b/drivers/usb/usbvideo.h
@@ -350,7 +350,7 @@ void usbvideo_CameraRelease(uvd_t *uvd);
 int usbvideo_v4l_close(struct inode *inode, struct file *file);
 int usbvideo_v4l_initialize(struct video_device *dev);
 int usbvideo_v4l_ioctl(struct inode *inode, struct file *file,
-                      unsigned int ioctlnr, void *arg);
+                      unsigned int cmd, unsigned long arg);
 int usbvideo_v4l_mmap(struct file *file, struct vm_area_struct *vma);
 int usbvideo_v4l_open(struct inode *inode, struct file *file);
 int usbvideo_v4l_read(struct file *file, char *buf,

diff --git a/include/linux/videodev.h b/include/linux/videodev.h
index 542e3ececef414568404f5b98dfa018a4c6f2a16..80966ed8e2885958c61a862f56ecd96d03ea67f5 100644 (file)
--- a/include/linux/videodev.h
+++ b/include/linux/videodev.h
@@ -37,8 +37,6 @@ struct video_device
         * video_generic_ioctl() does the userspace copying of the
         * ioctl arguments */
        struct file_operations *fops;
-       int (*kernel_ioctl)(struct inode *inode, struct file *file,
-                           unsigned int cmd, void *arg);
        void *priv;             /* Used to be 'private' but that upsets C++ */
 
        /* for videodev.c intenal usage -- don't touch */
@@ -60,8 +58,10 @@ extern struct video_device* video_devdata(struct file*);
 
 extern int video_exclusive_open(struct inode *inode, struct file *file);
 extern int video_exclusive_release(struct inode *inode, struct file *file);
-extern int video_generic_ioctl(struct inode *inode, struct file *file,
-                              unsigned int cmd, unsigned long arg);
+extern int video_usercopy(struct inode *inode, struct file *file,
+                         unsigned int cmd, unsigned long arg,
+                         int (*func)(struct inode *inode, struct file *file,
+                                     unsigned int cmd, void *arg));
 #endif /* __KERNEL__ */
 
 #define VID_TYPE_CAPTURE       1       /* Can capture */