iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

media: v4l2: prepare compat-ioctl rework

Browse Source 
The v4l2-compat-ioctl32() currently takes an extra round trip through user
space pointers when converting the data structure formats. In particular,
this involves using the compat_alloc_user_space() and copy_in_user()
helpers that often lead to worse compat handlers compared to using
in_compat_syscall() checks when copying the data.

The native implementation already gained a simpler method to deal with
the conversion for the time32 conversion.  Hook into the same places to
provide a location for reading and writing user space data from inside
of the generic video_usercopy() helper.

Hans Verkuil rewrote the video_get_user() function here to simplify
the zeroing of the extra input fields and fixed a couple of bugs in
the original implementation.

[hverkuil: fix: CHECK: Please don't use multiple blank lines]

Co-developed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
This commit is contained in:
Arnd Bergmann 2020-10-30 17:55:22 +01:00 committed by Mauro Carvalho Chehab
parent 3650b228f8
commit 8dbcc3fa38
3 changed files with 154 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
drivers/media/v4l2-core/v4l2-compat-ioctl32.c
View File

@ -1414,6 +1414,59 @@ static int put_v4l2_edid32(struct v4l2_edid __user *p64,
#define VIDIOC_G_OUTPUT32 _IOR ('V', 46, s32) #define VIDIOC_G_OUTPUT32 _IOR ('V', 46, s32)
#define VIDIOC_S_OUTPUT32 _IOWR('V', 47, s32) #define VIDIOC_S_OUTPUT32 _IOWR('V', 47, s32)
unsigned int v4l2_compat_translate_cmd(unsigned int cmd)
{
switch (cmd) {
}
return cmd;
}
int v4l2_compat_get_user(void __user *arg, void *parg, unsigned int cmd)
{
switch (cmd) {
}
return 0;
}
int v4l2_compat_put_user(void __user *arg, void *parg, unsigned int cmd)
{
switch (cmd) {
}
return 0;
}
int v4l2_compat_get_array_args(struct file *file, void *mbuf,
void __user *user_ptr, size_t array_size,
unsigned int cmd, void *arg)
{
int err = 0;
switch (cmd) {
default:
if (copy_from_user(mbuf, user_ptr, array_size))
err = -EFAULT;
break;
}
return err;
}
int v4l2_compat_put_array_args(struct file *file, void __user *user_ptr,
void *mbuf, size_t array_size,
unsigned int cmd, void *arg)
{
int err = 0;
switch (cmd) {
default:
if (copy_to_user(user_ptr, mbuf, array_size))
err = -EFAULT;
break;
}
return err;
}
/** /**
* alloc_userspace() - Allocates a 64-bits userspace pointer compatible * alloc_userspace() - Allocates a 64-bits userspace pointer compatible
* for calling the native 64-bits version of an ioctl. * for calling the native 64-bits version of an ioctl.

157
drivers/media/v4l2-core/v4l2-ioctl.c
View File

@ -8,6 +8,7 @@
* Mauro Carvalho Chehab <mchehab@kernel.org> (version 2) * Mauro Carvalho Chehab <mchehab@kernel.org> (version 2)
*/ */
#include <linux/compat.h>
#include <linux/mm.h> #include <linux/mm.h>
#include <linux/module.h> #include <linux/module.h>
#include <linux/slab.h> #include <linux/slab.h>
@ -3104,14 +3105,18 @@ static unsigned int video_translate_cmd(unsigned int cmd)
return VIDIOC_PREPARE_BUF; return VIDIOC_PREPARE_BUF;
#endif #endif
} }
if (in_compat_syscall())
return v4l2_compat_translate_cmd(cmd);
return cmd; return cmd;
} }
static int video_get_user(void __user *arg, void *parg, unsigned int cmd, static int video_get_user(void __user *arg, void *parg,
unsigned int real_cmd, unsigned int cmd,
bool *always_copy) bool *always_copy)
{ {
unsigned int n = _IOC_SIZE(cmd); unsigned int n = _IOC_SIZE(real_cmd);
int err = 0;
if (!(_IOC_DIR(cmd) & _IOC_WRITE)) { if (!(_IOC_DIR(cmd) & _IOC_WRITE)) {
/* read-only ioctl */ /* read-only ioctl */
@ -3119,73 +3124,82 @@ static int video_get_user(void __user *arg, void *parg, unsigned int cmd,
return 0; return 0;
} }
switch (cmd) { /*
#ifdef CONFIG_COMPAT_32BIT_TIME * In some cases, only a few fields are used as input,
case VIDIOC_QUERYBUF_TIME32: * i.e. when the app sets "index" and then the driver
case VIDIOC_QBUF_TIME32: * fills in the rest of the structure for the thing
case VIDIOC_DQBUF_TIME32: * with that index. We only need to copy up the first
case VIDIOC_PREPARE_BUF_TIME32: { * non-input field.
struct v4l2_buffer_time32 vb32; */
struct v4l2_buffer *vb = parg; if (v4l2_is_known_ioctl(real_cmd)) {
u32 flags = v4l2_ioctls[_IOC_NR(real_cmd)].flags;
if (copy_from_user(&vb32, arg, sizeof(vb32))) if (flags & INFO_FL_CLEAR_MASK)
return -EFAULT; n = (flags & INFO_FL_CLEAR_MASK) >> 16;
*always_copy = flags & INFO_FL_ALWAYS_COPY;
*vb = (struct v4l2_buffer) {
.index = vb32.index,
.type = vb32.type,
.bytesused = vb32.bytesused,
.flags = vb32.flags,
.field = vb32.field,
.timestamp.tv_sec = vb32.timestamp.tv_sec,
.timestamp.tv_usec = vb32.timestamp.tv_usec,
.timecode = vb32.timecode,
.sequence = vb32.sequence,
.memory = vb32.memory,
.m.userptr = vb32.m.userptr,
.length = vb32.length,
.request_fd = vb32.request_fd,
};
if (cmd == VIDIOC_QUERYBUF_TIME32)
vb->request_fd = 0;
break;
} }
#endif
default:
/*
* 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.
*/
if (v4l2_is_known_ioctl(cmd)) {
u32 flags = v4l2_ioctls[_IOC_NR(cmd)].flags;
if (flags & INFO_FL_CLEAR_MASK)
n = (flags & INFO_FL_CLEAR_MASK) >> 16;
*always_copy = flags & INFO_FL_ALWAYS_COPY;
}
if (cmd == real_cmd) {
if (copy_from_user(parg, (void __user *)arg, n)) if (copy_from_user(parg, (void __user *)arg, n))
return -EFAULT; err = -EFAULT;
} else if (in_compat_syscall()) {
err = v4l2_compat_get_user(arg, parg, cmd);
} else {
switch (cmd) {
#ifdef CONFIG_COMPAT_32BIT_TIME
case VIDIOC_QUERYBUF_TIME32:
case VIDIOC_QBUF_TIME32:
case VIDIOC_DQBUF_TIME32:
case VIDIOC_PREPARE_BUF_TIME32: {
struct v4l2_buffer_time32 vb32;
struct v4l2_buffer *vb = parg;
/* zero out anything we don't copy from userspace */ if (copy_from_user(&vb32, arg, sizeof(vb32)))
if (n < _IOC_SIZE(cmd)) return -EFAULT;
memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n);
break; *vb = (struct v4l2_buffer) {
.index = vb32.index,
.type = vb32.type,
.bytesused = vb32.bytesused,
.flags = vb32.flags,
.field = vb32.field,
.timestamp.tv_sec = vb32.timestamp.tv_sec,
.timestamp.tv_usec = vb32.timestamp.tv_usec,
.timecode = vb32.timecode,
.sequence = vb32.sequence,
.memory = vb32.memory,
.m.userptr = vb32.m.userptr,
.length = vb32.length,
.request_fd = vb32.request_fd,
};
break;
}
#endif
}
} }
return 0; /* zero out anything we don't copy from userspace */
if (!err && n < _IOC_SIZE(real_cmd))
memset((u8 *)parg + n, 0, _IOC_SIZE(real_cmd) - n);
return err;
} }
static int video_put_user(void __user *arg, void *parg, unsigned int cmd) static int video_put_user(void __user *arg, void *parg,
unsigned int real_cmd, unsigned int cmd)
{ {
if (!(_IOC_DIR(cmd) & _IOC_READ)) if (!(_IOC_DIR(cmd) & _IOC_READ))
return 0; return 0;
if (cmd == real_cmd) {
/* Copy results into user buffer */
if (copy_to_user(arg, parg, _IOC_SIZE(cmd)))
return -EFAULT;
return 0;
}
if (in_compat_syscall())
return v4l2_compat_put_user(arg, parg, cmd);
switch (cmd) { switch (cmd) {
#ifdef CONFIG_COMPAT_32BIT_TIME #ifdef CONFIG_COMPAT_32BIT_TIME
case VIDIOC_DQEVENT_TIME32: { case VIDIOC_DQEVENT_TIME32: {
@ -3236,11 +3250,6 @@ static int video_put_user(void __user *arg, void *parg, unsigned int cmd)
break; break;
} }
#endif #endif
default:
/* Copy results into user buffer */
if (copy_to_user(arg, parg, _IOC_SIZE(cmd)))
return -EFAULT;
break;
} }
return 0; return 0;
@ -3274,8 +3283,8 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
parg = mbuf; parg = mbuf;
} }
err = video_get_user((void __user *)arg, parg, orig_cmd, err = video_get_user((void __user *)arg, parg, cmd,
&always_copy); orig_cmd, &always_copy);
if (err) if (err)
goto out; goto out;
} }
@ -3297,7 +3306,14 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
if (NULL == mbuf) if (NULL == mbuf)
goto out_array_args; goto out_array_args;
err = -EFAULT; err = -EFAULT;
if (copy_from_user(mbuf, user_ptr, array_size)) if (in_compat_syscall())
err = v4l2_compat_get_array_args(file, mbuf, user_ptr,
array_size, orig_cmd,
parg);
else
err = copy_from_user(mbuf, user_ptr, array_size) ?
-EFAULT : 0;
if (err)
goto out_array_args; goto out_array_args;
*kernel_ptr = mbuf; *kernel_ptr = mbuf;
} }
@ -3318,8 +3334,17 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
if (has_array_args) { if (has_array_args) {
*kernel_ptr = (void __force *)user_ptr; *kernel_ptr = (void __force *)user_ptr;
if (copy_to_user(user_ptr, mbuf, array_size)) if (in_compat_syscall()) {
int put_err;
put_err = v4l2_compat_put_array_args(file, user_ptr, mbuf,
array_size, orig_cmd,
parg);
if (put_err)
err = put_err;
} else if (copy_to_user(user_ptr, mbuf, array_size)) {
err = -EFAULT; err = -EFAULT;
}
goto out_array_args; goto out_array_args;
} }
/* /*
@ -3330,7 +3355,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
goto out; goto out;
out_array_args: out_array_args:
if (video_put_user((void __user *)arg, parg, orig_cmd)) if (video_put_user((void __user *)arg, parg, cmd, orig_cmd))
err = -EFAULT; err = -EFAULT;
out: out:
kvfree(mbuf); kvfree(mbuf);

10
include/media/v4l2-ioctl.h
View File

@ -686,6 +686,16 @@ long int v4l2_compat_ioctl32(struct file *file, unsigned int cmd,
unsigned long arg); unsigned long arg);
#endif #endif
unsigned int v4l2_compat_translate_cmd(unsigned int cmd);
int v4l2_compat_get_user(void __user *arg, void *parg, unsigned int cmd);
int v4l2_compat_put_user(void __user *arg, void *parg, unsigned int cmd);
int v4l2_compat_get_array_args(struct file *file, void *mbuf,
void __user *user_ptr, size_t array_size,
unsigned int cmd, void *arg);
int v4l2_compat_put_array_args(struct file *file, void __user *user_ptr,
void *mbuf, size_t array_size,
unsigned int cmd, void *arg);
/** /**
* typedef v4l2_kioctl - Typedef used to pass an ioctl handler. * typedef v4l2_kioctl - Typedef used to pass an ioctl handler.
* *