diff options
author | Laurent Pinchart <laurent.pinchart@ideasonboard.com> | 2014-11-01 09:32:28 -0400 |
---|---|---|
committer | Mauro Carvalho Chehab <mchehab@osg.samsung.com> | 2014-12-04 09:41:11 -0500 |
commit | f6cee18858b3c0adaa30cbe45a1766c8ef053c5d (patch) | |
tree | 9b8da082484a7f90c4da7bc7e425f254ceb92c0f | |
parent | b518d86609cc066b626120fe6ec6fe3a4ccfcd54 (diff) |
[media] v4l: vb2: Fix race condition in vb2_fop_poll
The vb2_fop_poll() implementation tries to be clever on whether it needs
to lock the queue mutex by checking whether polling might start fileio.
The test requires reading the q->num_buffer field, which is racy if we
don't hold the queue mutex in the first place.
Remove the extra cleverness and just lock the mutex.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
-rw-r--r-- | drivers/media/v4l2-core/videobuf2-core.c | 27 |
1 files changed, 8 insertions, 19 deletions
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 7aed8f22e075..2685670b20ec 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c | |||
@@ -3459,27 +3459,16 @@ unsigned int vb2_fop_poll(struct file *file, poll_table *wait) | |||
3459 | struct video_device *vdev = video_devdata(file); | 3459 | struct video_device *vdev = video_devdata(file); |
3460 | struct vb2_queue *q = vdev->queue; | 3460 | struct vb2_queue *q = vdev->queue; |
3461 | struct mutex *lock = q->lock ? q->lock : vdev->lock; | 3461 | struct mutex *lock = q->lock ? q->lock : vdev->lock; |
3462 | unsigned long req_events = poll_requested_events(wait); | ||
3463 | unsigned res; | 3462 | unsigned res; |
3464 | void *fileio; | 3463 | void *fileio; |
3465 | bool must_lock = false; | ||
3466 | |||
3467 | /* Try to be smart: only lock if polling might start fileio, | ||
3468 | otherwise locking will only introduce unwanted delays. */ | ||
3469 | if (q->num_buffers == 0 && !vb2_fileio_is_active(q)) { | ||
3470 | if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) && | ||
3471 | (req_events & (POLLIN | POLLRDNORM))) | ||
3472 | must_lock = true; | ||
3473 | else if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE) && | ||
3474 | (req_events & (POLLOUT | POLLWRNORM))) | ||
3475 | must_lock = true; | ||
3476 | } | ||
3477 | 3464 | ||
3478 | /* If locking is needed, but this helper doesn't know how, then you | 3465 | /* |
3479 | shouldn't be using this helper but you should write your own. */ | 3466 | * If this helper doesn't know how to lock, then you shouldn't be using |
3480 | WARN_ON(must_lock && !lock); | 3467 | * it but you should write your own. |
3468 | */ | ||
3469 | WARN_ON(!lock); | ||
3481 | 3470 | ||
3482 | if (must_lock && lock && mutex_lock_interruptible(lock)) | 3471 | if (lock && mutex_lock_interruptible(lock)) |
3483 | return POLLERR; | 3472 | return POLLERR; |
3484 | 3473 | ||
3485 | fileio = q->fileio; | 3474 | fileio = q->fileio; |
@@ -3487,9 +3476,9 @@ unsigned int vb2_fop_poll(struct file *file, poll_table *wait) | |||
3487 | res = vb2_poll(vdev->queue, file, wait); | 3476 | res = vb2_poll(vdev->queue, file, wait); |
3488 | 3477 | ||
3489 | /* If fileio was started, then we have a new queue owner. */ | 3478 | /* If fileio was started, then we have a new queue owner. */ |
3490 | if (must_lock && !fileio && q->fileio) | 3479 | if (!fileio && q->fileio) |
3491 | q->owner = file->private_data; | 3480 | q->owner = file->private_data; |
3492 | if (must_lock && lock) | 3481 | if (lock) |
3493 | mutex_unlock(lock); | 3482 | mutex_unlock(lock); |
3494 | return res; | 3483 | return res; |
3495 | } | 3484 | } |