| Message ID | 1375725209-2674-5-git-send-email-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon 5 August 2013 19:53:23 Laurent Pinchart wrote: > The vb2_fop_mmap() function is a plug-in implementation of the mmap() > file operation that calls vb2_mmap() on the queue associated with the > video device. Neither the vb2_fop_mmap() function nor the v4l2_mmap() > mmap handler in the V4L2 core take any lock, leading to race conditions > between mmap() and other buffer-related ioctls such as VIDIOC_REQBUFS. > > Fix it by taking the queue or device lock around the vb2_mmap() call. Hi Laurent, Can you do the same for vb2_fop_get_unmapped_area()? Thanks! Hans > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > drivers/media/v4l2-core/videobuf2-core.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c > index 9fc4bab..bd4bade 100644 > --- a/drivers/media/v4l2-core/videobuf2-core.c > +++ b/drivers/media/v4l2-core/videobuf2-core.c > @@ -2578,8 +2578,15 @@ EXPORT_SYMBOL_GPL(vb2_ioctl_expbuf); > int vb2_fop_mmap(struct file *file, struct vm_area_struct *vma) > { > struct video_device *vdev = video_devdata(file); > + struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock; > + int err; > > - return vb2_mmap(vdev->queue, vma); > + if (lock && mutex_lock_interruptible(lock)) > + return -ERESTARTSYS; > + err = vb2_mmap(vdev->queue, vma); > + if (lock) > + mutex_unlock(lock); > + return err; > } > EXPORT_SYMBOL_GPL(vb2_fop_mmap); > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Hans, On Tuesday 06 August 2013 12:39:27 Hans Verkuil wrote: > On Mon 5 August 2013 19:53:23 Laurent Pinchart wrote: > > The vb2_fop_mmap() function is a plug-in implementation of the mmap() > > file operation that calls vb2_mmap() on the queue associated with the > > video device. Neither the vb2_fop_mmap() function nor the v4l2_mmap() > > mmap handler in the V4L2 core take any lock, leading to race conditions > > between mmap() and other buffer-related ioctls such as VIDIOC_REQBUFS. > > > > Fix it by taking the queue or device lock around the vb2_mmap() call. > > Hi Laurent, > > Can you do the same for vb2_fop_get_unmapped_area()? Sure. I'll repost a v7 of this patch that fixes both mmap and get_unmapped_area. > > Signed-off-by: Laurent Pinchart > > <laurent.pinchart+renesas@ideasonboard.com> > > --- > > > > drivers/media/v4l2-core/videobuf2-core.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c > > b/drivers/media/v4l2-core/videobuf2-core.c index 9fc4bab..bd4bade 100644 > > --- a/drivers/media/v4l2-core/videobuf2-core.c > > +++ b/drivers/media/v4l2-core/videobuf2-core.c > > @@ -2578,8 +2578,15 @@ EXPORT_SYMBOL_GPL(vb2_ioctl_expbuf); > > > > int vb2_fop_mmap(struct file *file, struct vm_area_struct *vma) > > { > > > > struct video_device *vdev = video_devdata(file); > > > > + struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev- >lock; > > + int err; > > > > - return vb2_mmap(vdev->queue, vma); > > + if (lock && mutex_lock_interruptible(lock)) > > + return -ERESTARTSYS; > > + err = vb2_mmap(vdev->queue, vma); > > + if (lock) > > + mutex_unlock(lock); > > + return err; > > > > } > > EXPORT_SYMBOL_GPL(vb2_fop_mmap);
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 9fc4bab..bd4bade 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -2578,8 +2578,15 @@ EXPORT_SYMBOL_GPL(vb2_ioctl_expbuf); int vb2_fop_mmap(struct file *file, struct vm_area_struct *vma) { struct video_device *vdev = video_devdata(file); + struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock; + int err; - return vb2_mmap(vdev->queue, vma); + if (lock && mutex_lock_interruptible(lock)) + return -ERESTARTSYS; + err = vb2_mmap(vdev->queue, vma); + if (lock) + mutex_unlock(lock); + return err; } EXPORT_SYMBOL_GPL(vb2_fop_mmap);
The vb2_fop_mmap() function is a plug-in implementation of the mmap() file operation that calls vb2_mmap() on the queue associated with the video device. Neither the vb2_fop_mmap() function nor the v4l2_mmap() mmap handler in the V4L2 core take any lock, leading to race conditions between mmap() and other buffer-related ioctls such as VIDIOC_REQBUFS. Fix it by taking the queue or device lock around the vb2_mmap() call. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/media/v4l2-core/videobuf2-core.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)