From 3851523aa0a9fedad28747dff951ef4ea07f3b81 Mon Sep 17 00:00:00 2001 From: Alexander Viro Date: Mon, 12 Jul 2004 21:00:16 -0700 Subject: [PATCH] [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 | 130 ++++++++++++++++++++++------------------------ 1 file changed, 61 insertions(+), 69 deletions(-) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 5b2198974cdf0..41e458c3a9981 100644 --- 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) -- 2.53.0