[PATCH] sparse: VIDIOCSWIN compat_ioctl fixes

In handling of VIDIOCSWIN for 32bit on 64bit platforms:
	* switched to compat_alloc_user_space()
	* fixed memory corruption in copying arguments from userland
	* fixed arithmetic overflows
	* added missing checks for get_user() results
	  and corresponding returns with -EFAULT.

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 5b2198974cdf03922af01f50535d6bf9f863ff03..41e458c3a99819a59ff0cce7a1a5193574c95e0f 100644 (file)
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -257,66 +257,6 @@ struct video_window32 {
        compat_int_t clipcount;
 };
 
-static void free_kvideo_clips(struct video_window *kp)
-{
-       struct video_clip *cp;
-
-       cp = kp->clips;
-       if(cp != NULL)
-               kfree(cp);
-}
-
-static int get_video_window32(struct video_window *kp, struct video_window32 __user *up)
-{
-       struct video_clip32 __user *ucp;
-       struct video_clip *kcp;
-       int nclips, err, i;
-       u32 tmp;
-
-       if(get_user(kp->x, &up->x))
-               return -EFAULT;
-       __get_user(kp->y, &up->y);
-       __get_user(kp->width, &up->width);
-       __get_user(kp->height, &up->height);
-       __get_user(kp->chromakey, &up->chromakey);
-       __get_user(kp->flags, &up->flags);
-       __get_user(kp->clipcount, &up->clipcount);
-       __get_user(tmp, &up->clips);
-       ucp = compat_ptr(tmp);
-       kp->clips = NULL;
-
-       nclips = kp->clipcount;
-       if(nclips == 0)
-               return 0;
-
-       if(ucp == 0)
-               return -EINVAL;
-
-       /* Peculiar interface... */
-       if(nclips < 0)
-               nclips = VIDEO_CLIPMAP_SIZE;
-
-       kcp = kmalloc(nclips * sizeof(struct video_clip), GFP_KERNEL);
-       err = -ENOMEM;
-       if(kcp == NULL)
-               goto cleanup_and_err;
-
-       kp->clips = kcp;
-       for(i = 0; i < nclips; i++) {
-               __get_user(kcp[i].x, &ucp[i].x);
-               __get_user(kcp[i].y, &ucp[i].y);
-               __get_user(kcp[i].width, &ucp[i].width);
-               __get_user(kcp[i].height, &ucp[i].height);
-               kcp[nclips].next = NULL;
-       }
-
-       return 0;
-
-cleanup_and_err:
-       free_kvideo_clips(kp);
-       return err;
-}
-
 /* You get back everything except the clips... */
 static int put_video_window32(struct video_window *kp, struct video_window32 __user *up)
 {
@@ -340,6 +280,66 @@ static int put_video_window32(struct video_window *kp, struct video_window32 __u
 #define VIDIOCGFREQ32          _IOR('v',14, u32)
 #define VIDIOCSFREQ32          _IOW('v',15, u32)
 
+enum {
+       MaxClips = (~0U-sizeof(struct video_window))/sizeof(struct video_clip)
+};
+
+static int do_set_window(unsigned int fd, unsigned int cmd, unsigned long arg)
+{
+       struct video_window32 __user *up = compat_ptr(arg);
+       struct video_window __user *vw;
+       struct video_clip __user *p;
+       int nclips;
+       u32 n;
+
+       if (get_user(nclips, &up->clipcount))
+               return -EFAULT;
+
+       /* Peculiar interface... */
+       if (nclips < 0)
+               nclips = VIDEO_CLIPMAP_SIZE;
+
+       if (nclips > MaxClips)
+               return -ENOMEM;
+
+       vw = compat_alloc_user_space(sizeof(struct video_window) +
+                                   nclips * sizeof(struct video_clip));
+
+       p = nclips ? (struct video_clip __user *)(vw + 1) : NULL;
+
+       if (get_user(n, &up->x) || put_user(n, &vw->x) ||
+           get_user(n, &up->y) || put_user(n, &vw->y) ||
+           get_user(n, &up->width) || put_user(n, &vw->width) ||
+           get_user(n, &up->height) || put_user(n, &vw->height) ||
+           get_user(n, &up->chromakey) || put_user(n, &vw->chromakey) ||
+           get_user(n, &up->flags) || put_user(n, &vw->flags) ||
+           get_user(n, &up->clipcount) || put_user(n, &vw->clipcount) ||
+           get_user(n, &up->clips) || put_user(p, &vw->clips))
+               return -EFAULT;
+
+       if (nclips) {
+               struct video_clip32 __user *u = compat_ptr(n);
+               int i;
+               if (!u)
+                       return -EINVAL;
+               for (i = 0; i < nclips; i++, u++, p++) {
+                       s32 v;
+                       if (get_user(v, &u->x) ||
+                           put_user(v, &p->x) ||
+                           get_user(v, &u->y) ||
+                           put_user(v, &p->y) ||
+                           get_user(v, &u->width) ||
+                           put_user(v, &p->width) ||
+                           get_user(v, &u->height) ||
+                           put_user(v, &p->height) ||
+                           put_user(NULL, &p->next))
+                               return -EFAULT;
+               }
+       }
+
+       return sys_ioctl(fd, VIDIOCSWIN, (unsigned long)p);
+}
+
 static int do_video_ioctl(unsigned int fd, unsigned int cmd, unsigned long arg)
 {
        union {
@@ -357,7 +357,6 @@ static int do_video_ioctl(unsigned int fd, unsigned int cmd, unsigned long arg)
        case VIDIOCGTUNER32: cmd = VIDIOCGTUNER; break;
        case VIDIOCSTUNER32: cmd = VIDIOCSTUNER; break;
        case VIDIOCGWIN32: cmd = VIDIOCGWIN; break;
-       case VIDIOCSWIN32: cmd = VIDIOCSWIN; break;
        case VIDIOCGFBUF32: cmd = VIDIOCGFBUF; break;
        case VIDIOCSFBUF32: cmd = VIDIOCSFBUF; break;
        case VIDIOCGFREQ32: cmd = VIDIOCGFREQ; break;
@@ -370,10 +369,6 @@ static int do_video_ioctl(unsigned int fd, unsigned int cmd, unsigned long arg)
                err = get_video_tuner32(&karg.vt, up);
                break;
 
-       case VIDIOCSWIN:
-               err = get_video_window32(&karg.vw, up);
-               break;
-
        case VIDIOCSFBUF:
                err = get_video_buffer32(&karg.vb, up);
                break;
@@ -389,9 +384,6 @@ static int do_video_ioctl(unsigned int fd, unsigned int cmd, unsigned long arg)
        err = sys_ioctl(fd, cmd, (unsigned long)&karg);
        set_fs(old_fs);
 
-       if(cmd == VIDIOCSWIN)
-               free_kvideo_clips(&karg.vw);
-
        if(err == 0) {
                switch(cmd) {
                case VIDIOCGTUNER:
@@ -3342,7 +3334,7 @@ HANDLE_IOCTL(EXT2_IOC32_SETVERSION, do_ext2_ioctl)
 HANDLE_IOCTL(VIDIOCGTUNER32, do_video_ioctl)
 HANDLE_IOCTL(VIDIOCSTUNER32, do_video_ioctl)
 HANDLE_IOCTL(VIDIOCGWIN32, do_video_ioctl)
-HANDLE_IOCTL(VIDIOCSWIN32, do_video_ioctl)
+HANDLE_IOCTL(VIDIOCSWIN32, do_set_window)
 HANDLE_IOCTL(VIDIOCGFBUF32, do_video_ioctl)
 HANDLE_IOCTL(VIDIOCSFBUF32, do_video_ioctl)
 HANDLE_IOCTL(VIDIOCGFREQ32, do_video_ioctl)