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
