Hello,
On 8/9/2013 2:11 PM, 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>
Acked-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
---
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);
Best regards
--
Marek Szyprowski
Samsung R&D Institute Poland
--
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
