[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.

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)