Skip to content

Commit

Permalink
media: v4l2-core: push taking ioctl mutex down to ioctl handler
Browse files Browse the repository at this point in the history
The ioctl serialization mutex (vdev->lock or q->lock for vb2 queues)
was taken at the highest level in v4l2-dev.c. This prevents more
fine-grained locking since at that level we cannot examine the ioctl
arguments, we can only do that after video_usercopy is called.

So push the locking down to __video_do_ioctl() and subdev_do_ioctl_lock().

This also allows us to make a few functions in v4l2-ioctl.c static and
video_usercopy() is no longer exported.

The locking scheme is not changed by this patch, just pushed down.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
  • Loading branch information
Hans Verkuil authored and Mauro Carvalho Chehab committed May 28, 2018
1 parent eba09b5 commit 73a1106
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 31 deletions.
6 changes: 0 additions & 6 deletions drivers/media/v4l2-core/v4l2-dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -360,14 +360,8 @@ static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
int ret = -ENODEV;

if (vdev->fops->unlocked_ioctl) {
struct mutex *lock = v4l2_ioctl_get_lock(vdev, cmd);

if (lock && mutex_lock_interruptible(lock))
return -ERESTARTSYS;
if (video_is_registered(vdev))
ret = vdev->fops->unlocked_ioctl(filp, cmd, arg);
if (lock)
mutex_unlock(lock);
} else
ret = -ENOTTY;

Expand Down
20 changes: 17 additions & 3 deletions drivers/media/v4l2-core/v4l2-ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2655,14 +2655,15 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
};
#define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)

bool v4l2_is_known_ioctl(unsigned int cmd)
static bool v4l2_is_known_ioctl(unsigned int cmd)
{
if (_IOC_NR(cmd) >= V4L2_IOCTLS)
return false;
return v4l2_ioctls[_IOC_NR(cmd)].ioctl == cmd;
}

struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned cmd)
static struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev,
unsigned int cmd)
{
if (_IOC_NR(cmd) >= V4L2_IOCTLS)
return vdev->lock;
Expand Down Expand Up @@ -2713,6 +2714,7 @@ static long __video_do_ioctl(struct file *file,
unsigned int cmd, void *arg)
{
struct video_device *vfd = video_devdata(file);
struct mutex *lock; /* ioctl serialization mutex */
const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
bool write_only = false;
struct v4l2_ioctl_info default_info;
Expand All @@ -2731,6 +2733,16 @@ static long __video_do_ioctl(struct file *file,
if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags))
vfh = file->private_data;

lock = v4l2_ioctl_get_lock(vfd, cmd);

if (lock && mutex_lock_interruptible(lock))
return -ERESTARTSYS;

if (!video_is_registered(vfd)) {
ret = -ENODEV;
goto unlock;
}

if (v4l2_is_known_ioctl(cmd)) {
info = &v4l2_ioctls[_IOC_NR(cmd)];

Expand Down Expand Up @@ -2780,6 +2792,9 @@ static long __video_do_ioctl(struct file *file,
}
}

unlock:
if (lock)
mutex_unlock(lock);
return ret;
}

Expand Down Expand Up @@ -2969,7 +2984,6 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
kvfree(mbuf);
return err;
}
EXPORT_SYMBOL(video_usercopy);

long video_ioctl2(struct file *file,
unsigned int cmd, unsigned long arg)
Expand Down
17 changes: 16 additions & 1 deletion drivers/media/v4l2-core/v4l2-subdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -502,10 +502,25 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
return 0;
}

static long subdev_do_ioctl_lock(struct file *file, unsigned int cmd, void *arg)
{
struct video_device *vdev = video_devdata(file);
struct mutex *lock = vdev->lock;
long ret = -ENODEV;

if (lock && mutex_lock_interruptible(lock))
return -ERESTARTSYS;
if (video_is_registered(vdev))
ret = subdev_do_ioctl(file, cmd, arg);
if (lock)
mutex_unlock(lock);
return ret;
}

static long subdev_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
return video_usercopy(file, cmd, arg, subdev_do_ioctl);
return video_usercopy(file, cmd, arg, subdev_do_ioctl_lock);
}

#ifdef CONFIG_COMPAT
Expand Down
9 changes: 0 additions & 9 deletions include/media/v4l2-dev.h
Original file line number Diff line number Diff line change
Expand Up @@ -435,15 +435,6 @@ void video_device_release(struct video_device *vdev);
*/
void video_device_release_empty(struct video_device *vdev);

/**
* v4l2_is_known_ioctl - Checks if a given cmd is a known V4L ioctl
*
* @cmd: ioctl command
*
* returns true if cmd is a known V4L2 ioctl
*/
bool v4l2_is_known_ioctl(unsigned int cmd);

/**
* v4l2_disable_ioctl- mark that a given command isn't implemented.
* shouldn't use core locking
Expand Down
12 changes: 0 additions & 12 deletions include/media/v4l2-ioctl.h
Original file line number Diff line number Diff line change
Expand Up @@ -658,18 +658,6 @@ void v4l_printk_ioctl(const char *prefix, unsigned int cmd);

struct video_device;


/**
* v4l2_ioctl_get_lock - get the mutex (if any) that it is need to lock for
* a given command.
*
* @vdev: Pointer to struct &video_device.
* @cmd: Ioctl name.
*
* .. note:: Internal use only. Should not be used outside V4L2 core.
*/
struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned int cmd);

/* names for fancy debug output */
extern const char *v4l2_field_names[];
extern const char *v4l2_type_names[];
Expand Down

0 comments on commit 73a1106

Please sign in to comment.