On Monday 12 July 2010 17:25:46 Laurent Pinchart wrote: > The two functions are mostly identical. They handle the copy_from_user > and copy_to_user operations related with V4L2 ioctls and call the real > ioctl handler. > > Create a __video_usercopy function that implements the core of > video_usercopy and video_ioctl2, and call that function from both.
Acked-by: Hans Verkuil <hverk...@xs4all.nl> Two notes: 1) This change will clash with the multiplane patches. 2) Perhaps it is time that we remove the __OLD_VIDIOC_ support? Regards, Hans > > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > --- > drivers/media/video/v4l2-ioctl.c | 218 ++++++++++++------------------------- > 1 files changed, 71 insertions(+), 147 deletions(-) > > diff --git a/drivers/media/video/v4l2-ioctl.c > b/drivers/media/video/v4l2-ioctl.c > index 0eeceae..486eaba 100644 > --- a/drivers/media/video/v4l2-ioctl.c > +++ b/drivers/media/video/v4l2-ioctl.c > @@ -373,35 +373,62 @@ video_fix_command(unsigned int cmd) > } > #endif > > -/* > - * Obsolete usercopy function - Should be removed soon > - */ > -long > -video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, > +/* In some cases, only a few fields are used as input, i.e. when the app sets > + * "index" and then the driver fills in the rest of the structure for the > thing > + * with that index. We only need to copy up the first non-input field. */ > +static unsigned long cmd_input_size(unsigned int cmd) > +{ > + /* Size of structure up to and including 'field' */ > +#define CMDINSIZE(cmd, type, field) \ > + case VIDIOC_##cmd: \ > + return offsetof(struct v4l2_##type, field) + \ > + sizeof(((struct v4l2_##type *)0)->field); > + > + switch (cmd) { > + CMDINSIZE(ENUM_FMT, fmtdesc, type); > + CMDINSIZE(G_FMT, format, type); > + CMDINSIZE(QUERYBUF, buffer, type); > + CMDINSIZE(G_PARM, streamparm, type); > + CMDINSIZE(ENUMSTD, standard, index); > + CMDINSIZE(ENUMINPUT, input, index); > + CMDINSIZE(G_CTRL, control, id); > + CMDINSIZE(G_TUNER, tuner, index); > + CMDINSIZE(QUERYCTRL, queryctrl, id); > + CMDINSIZE(QUERYMENU, querymenu, index); > + CMDINSIZE(ENUMOUTPUT, output, index); > + CMDINSIZE(G_MODULATOR, modulator, index); > + CMDINSIZE(G_FREQUENCY, frequency, tuner); > + CMDINSIZE(CROPCAP, cropcap, type); > + CMDINSIZE(G_CROP, crop, type); > + CMDINSIZE(ENUMAUDIO, audio, index); > + CMDINSIZE(ENUMAUDOUT, audioout, index); > + CMDINSIZE(ENCODER_CMD, encoder_cmd, flags); > + CMDINSIZE(TRY_ENCODER_CMD, encoder_cmd, flags); > + CMDINSIZE(G_SLICED_VBI_CAP, sliced_vbi_cap, type); > + CMDINSIZE(ENUM_FRAMESIZES, frmsizeenum, pixel_format); > + CMDINSIZE(ENUM_FRAMEINTERVALS, frmivalenum, height); > + default: > + return _IOC_SIZE(cmd); > + } > +} > + > +static long > +__video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, > v4l2_kioctl func) > { > char sbuf[128]; > void *mbuf = NULL; > - void *parg = NULL; > + void *parg = (void *)arg; > long err = -EINVAL; > int is_ext_ctrl; > size_t ctrls_size = 0; > void __user *user_ptr = NULL; > > -#ifdef __OLD_VIDIOC_ > - cmd = video_fix_command(cmd); > -#endif > is_ext_ctrl = (cmd == VIDIOC_S_EXT_CTRLS || cmd == VIDIOC_G_EXT_CTRLS || > cmd == VIDIOC_TRY_EXT_CTRLS); > > /* Copy arguments into temp kernel buffer */ > - switch (_IOC_DIR(cmd)) { > - case _IOC_NONE: > - parg = NULL; > - break; > - case _IOC_READ: > - case _IOC_WRITE: > - case (_IOC_WRITE | _IOC_READ): > + if (_IOC_DIR(cmd) != _IOC_NONE) { > if (_IOC_SIZE(cmd) <= sizeof(sbuf)) { > parg = sbuf; > } else { > @@ -413,11 +440,21 @@ video_usercopy(struct file *file, unsigned int cmd, > unsigned long arg, > } > > err = -EFAULT; > - if (_IOC_DIR(cmd) & _IOC_WRITE) > - if (copy_from_user(parg, (void __user *)arg, > _IOC_SIZE(cmd))) > + if (_IOC_DIR(cmd) & _IOC_WRITE) { > + unsigned long n = cmd_input_size(cmd); > + > + if (copy_from_user(parg, (void __user *)arg, n)) > goto out; > - break; > + > + /* zero out anything we don't copy from userspace */ > + if (n < _IOC_SIZE(cmd)) > + memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n); > + } else { > + /* read-only ioctl */ > + memset(parg, 0, _IOC_SIZE(cmd)); > + } > } > + > if (is_ext_ctrl) { > struct v4l2_ext_controls *p = parg; > > @@ -439,7 +476,7 @@ video_usercopy(struct file *file, unsigned int cmd, > unsigned long arg, > } > } > > - /* call driver */ > + /* Handles IOCTL */ > err = func(file, cmd, parg); > if (err == -ENOIOCTLCMD) > err = -EINVAL; > @@ -468,6 +505,19 @@ out: > kfree(mbuf); > return err; > } > + > +/* > + * Obsolete usercopy function - Should be removed soon > + */ > +long > +video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, > + v4l2_kioctl func) > +{ > +#ifdef __OLD_VIDIOC_ > + cmd = video_fix_command(cmd); > +#endif > + return __video_usercopy(file, cmd, arg, func); > +} > EXPORT_SYMBOL(video_usercopy); > > static void dbgbuf(unsigned int cmd, struct video_device *vfd, > @@ -2021,138 +2071,12 @@ static long __video_do_ioctl(struct file *file, > return ret; > } > > -/* In some cases, only a few fields are used as input, i.e. when the app sets > - * "index" and then the driver fills in the rest of the structure for the > thing > - * with that index. We only need to copy up the first non-input field. */ > -static unsigned long cmd_input_size(unsigned int cmd) > -{ > - /* Size of structure up to and including 'field' */ > -#define CMDINSIZE(cmd, type, field) \ > - case VIDIOC_##cmd: \ > - return offsetof(struct v4l2_##type, field) + \ > - sizeof(((struct v4l2_##type *)0)->field); > - > - switch (cmd) { > - CMDINSIZE(ENUM_FMT, fmtdesc, type); > - CMDINSIZE(G_FMT, format, type); > - CMDINSIZE(QUERYBUF, buffer, type); > - CMDINSIZE(G_PARM, streamparm, type); > - CMDINSIZE(ENUMSTD, standard, index); > - CMDINSIZE(ENUMINPUT, input, index); > - CMDINSIZE(G_CTRL, control, id); > - CMDINSIZE(G_TUNER, tuner, index); > - CMDINSIZE(QUERYCTRL, queryctrl, id); > - CMDINSIZE(QUERYMENU, querymenu, index); > - CMDINSIZE(ENUMOUTPUT, output, index); > - CMDINSIZE(G_MODULATOR, modulator, index); > - CMDINSIZE(G_FREQUENCY, frequency, tuner); > - CMDINSIZE(CROPCAP, cropcap, type); > - CMDINSIZE(G_CROP, crop, type); > - CMDINSIZE(ENUMAUDIO, audio, index); > - CMDINSIZE(ENUMAUDOUT, audioout, index); > - CMDINSIZE(ENCODER_CMD, encoder_cmd, flags); > - CMDINSIZE(TRY_ENCODER_CMD, encoder_cmd, flags); > - CMDINSIZE(G_SLICED_VBI_CAP, sliced_vbi_cap, type); > - CMDINSIZE(ENUM_FRAMESIZES, frmsizeenum, pixel_format); > - CMDINSIZE(ENUM_FRAMEINTERVALS, frmivalenum, height); > - default: > - return _IOC_SIZE(cmd); > - } > -} > - > long video_ioctl2(struct file *file, > unsigned int cmd, unsigned long arg) > { > - char sbuf[128]; > - void *mbuf = NULL; > - void *parg = (void *)arg; > - long err = -EINVAL; > - int is_ext_ctrl; > - size_t ctrls_size = 0; > - void __user *user_ptr = NULL; > - > #ifdef __OLD_VIDIOC_ > cmd = video_fix_command(cmd); > #endif > - is_ext_ctrl = (cmd == VIDIOC_S_EXT_CTRLS || cmd == VIDIOC_G_EXT_CTRLS || > - cmd == VIDIOC_TRY_EXT_CTRLS); > - > - /* Copy arguments into temp kernel buffer */ > - if (_IOC_DIR(cmd) != _IOC_NONE) { > - if (_IOC_SIZE(cmd) <= sizeof(sbuf)) { > - parg = sbuf; > - } else { > - /* too big to allocate from stack */ > - mbuf = kmalloc(_IOC_SIZE(cmd), GFP_KERNEL); > - if (NULL == mbuf) > - return -ENOMEM; > - parg = mbuf; > - } > - > - err = -EFAULT; > - if (_IOC_DIR(cmd) & _IOC_WRITE) { > - unsigned long n = cmd_input_size(cmd); > - > - if (copy_from_user(parg, (void __user *)arg, n)) > - goto out; > - > - /* zero out anything we don't copy from userspace */ > - if (n < _IOC_SIZE(cmd)) > - memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n); > - } else { > - /* read-only ioctl */ > - memset(parg, 0, _IOC_SIZE(cmd)); > - } > - } > - > - if (is_ext_ctrl) { > - struct v4l2_ext_controls *p = parg; > - > - /* In case of an error, tell the caller that it wasn't > - a specific control that caused it. */ > - p->error_idx = p->count; > - user_ptr = (void __user *)p->controls; > - if (p->count) { > - ctrls_size = sizeof(struct v4l2_ext_control) * p->count; > - /* Note: v4l2_ext_controls fits in sbuf[] so mbuf is > still NULL. */ > - mbuf = kmalloc(ctrls_size, GFP_KERNEL); > - err = -ENOMEM; > - if (NULL == mbuf) > - goto out_ext_ctrl; > - err = -EFAULT; > - if (copy_from_user(mbuf, user_ptr, ctrls_size)) > - goto out_ext_ctrl; > - p->controls = mbuf; > - } > - } > - > - /* Handles IOCTL */ > - err = __video_do_ioctl(file, cmd, parg); > - if (err == -ENOIOCTLCMD) > - err = -EINVAL; > - if (is_ext_ctrl) { > - struct v4l2_ext_controls *p = parg; > - > - p->controls = (void *)user_ptr; > - if (p->count && err == 0 && copy_to_user(user_ptr, mbuf, > ctrls_size)) > - err = -EFAULT; > - goto out_ext_ctrl; > - } > - if (err < 0) > - goto out; > - > -out_ext_ctrl: > - /* Copy results into user buffer */ > - switch (_IOC_DIR(cmd)) { > - case _IOC_READ: > - case (_IOC_WRITE | _IOC_READ): > - if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd))) > - err = -EFAULT; > - break; > - } > - > -out: > - kfree(mbuf); > - return err; > + return __video_usercopy(file, cmd, arg, __video_do_ioctl); > } > EXPORT_SYMBOL(video_ioctl2); > -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html