On Fri 9 August 2013 14:11:25 Laurent Pinchart wrote:
> Commit b037c0fde22b1d3cd0b3c3717d28e54619fc1592 ("media: vb2: fix
> potential deadlock in mmap vs. get_userptr handling") fixes an AB-BA
> deadlock related to the mmap_sem and driver locks. The same deadlock can
> occur in vb2_prepare_buffer(), fix it the same way.
>
> Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Reviewed-by: Hans Verkuil <hans.verk...@cisco.com>
Regards,
Hans
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 52
> ++++++++++++++++++++++++++------
> 1 file changed, 43 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index 7f32860..7c2a8ce 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1248,50 +1248,84 @@ static int __buf_prepare(struct vb2_buffer *vb, const
> struct v4l2_buffer *b)
> */
> int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
> {
> + struct rw_semaphore *mmap_sem = NULL;
> struct vb2_buffer *vb;
> int ret;
>
> + /*
> + * In case of user pointer buffers vb2 allocator needs to get direct
> + * access to userspace pages. This requires getting read access on
> + * mmap semaphore in the current process structure. The same
> + * semaphore is taken before calling mmap operation, while both mmap
> + * and prepare_buf are called by the driver or v4l2 core with driver's
> + * lock held. To avoid a AB-BA deadlock (mmap_sem then driver's lock in
> + * mmap and driver's lock then mmap_sem in prepare_buf) the videobuf2
> + * core release driver's lock, takes mmap_sem and then takes again
> + * driver's lock.
> + *
> + * To avoid race with other vb2 calls, which might be called after
> + * releasing driver's lock, this operation is performed at the
> + * beggining of prepare_buf processing. This way the queue status is
> + * consistent after getting driver's lock back.
> + */
> + if (q->memory == V4L2_MEMORY_USERPTR) {
> + mmap_sem = ¤t->mm->mmap_sem;
> + call_qop(q, wait_prepare, q);
> + down_read(mmap_sem);
> + call_qop(q, wait_finish, q);
> + }
> +
> if (q->fileio) {
> dprintk(1, "%s(): file io in progress\n", __func__);
> - return -EBUSY;
> + ret = -EBUSY;
> + goto unlock;
> }
>
> if (b->type != q->type) {
> dprintk(1, "%s(): invalid buffer type\n", __func__);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto unlock;
> }
>
> if (b->index >= q->num_buffers) {
> dprintk(1, "%s(): buffer index out of range\n", __func__);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto unlock;
> }
>
> vb = q->bufs[b->index];
> if (NULL == vb) {
> /* Should never happen */
> dprintk(1, "%s(): buffer is NULL\n", __func__);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto unlock;
> }
>
> if (b->memory != q->memory) {
> dprintk(1, "%s(): invalid memory type\n", __func__);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto unlock;
> }
>
> if (vb->state != VB2_BUF_STATE_DEQUEUED) {
> dprintk(1, "%s(): invalid buffer state %d\n", __func__,
> vb->state);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto unlock;
> }
> ret = __verify_planes_array(vb, b);
> if (ret < 0)
> - return ret;
> + goto unlock;
> +
> ret = __buf_prepare(vb, b);
> if (ret < 0)
> - return ret;
> + goto unlock;
>
> __fill_v4l2_buffer(vb, b);
>
> - return 0;
> +unlock:
> + if (mmap_sem)
> + up_read(mmap_sem);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(vb2_prepare_buf);
>
>
--
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
