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.pinchart@xxxxxxxxxxxxxxxx> Reviewed-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html