Skip to content

Commit a517cca

Browse files
pinchartlmchehab
authored andcommitted
[media] media: vb2: Fix potential deadlock in vb2_prepare_buffer
Commit b037c0f ("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@ideasonboard.com> Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
1 parent 1ac7fde commit a517cca

File tree

1 file changed

+43
-9
lines changed

1 file changed

+43
-9
lines changed

drivers/media/v4l2-core/videobuf2-core.c

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,50 +1248,84 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
12481248
*/
12491249
int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
12501250
{
1251+
struct rw_semaphore *mmap_sem = NULL;
12511252
struct vb2_buffer *vb;
12521253
int ret;
12531254

1255+
/*
1256+
* In case of user pointer buffers vb2 allocator needs to get direct
1257+
* access to userspace pages. This requires getting read access on
1258+
* mmap semaphore in the current process structure. The same
1259+
* semaphore is taken before calling mmap operation, while both mmap
1260+
* and prepare_buf are called by the driver or v4l2 core with driver's
1261+
* lock held. To avoid a AB-BA deadlock (mmap_sem then driver's lock in
1262+
* mmap and driver's lock then mmap_sem in prepare_buf) the videobuf2
1263+
* core release driver's lock, takes mmap_sem and then takes again
1264+
* driver's lock.
1265+
*
1266+
* To avoid race with other vb2 calls, which might be called after
1267+
* releasing driver's lock, this operation is performed at the
1268+
* beggining of prepare_buf processing. This way the queue status is
1269+
* consistent after getting driver's lock back.
1270+
*/
1271+
if (q->memory == V4L2_MEMORY_USERPTR) {
1272+
mmap_sem = &current->mm->mmap_sem;
1273+
call_qop(q, wait_prepare, q);
1274+
down_read(mmap_sem);
1275+
call_qop(q, wait_finish, q);
1276+
}
1277+
12541278
if (q->fileio) {
12551279
dprintk(1, "%s(): file io in progress\n", __func__);
1256-
return -EBUSY;
1280+
ret = -EBUSY;
1281+
goto unlock;
12571282
}
12581283

12591284
if (b->type != q->type) {
12601285
dprintk(1, "%s(): invalid buffer type\n", __func__);
1261-
return -EINVAL;
1286+
ret = -EINVAL;
1287+
goto unlock;
12621288
}
12631289

12641290
if (b->index >= q->num_buffers) {
12651291
dprintk(1, "%s(): buffer index out of range\n", __func__);
1266-
return -EINVAL;
1292+
ret = -EINVAL;
1293+
goto unlock;
12671294
}
12681295

12691296
vb = q->bufs[b->index];
12701297
if (NULL == vb) {
12711298
/* Should never happen */
12721299
dprintk(1, "%s(): buffer is NULL\n", __func__);
1273-
return -EINVAL;
1300+
ret = -EINVAL;
1301+
goto unlock;
12741302
}
12751303

12761304
if (b->memory != q->memory) {
12771305
dprintk(1, "%s(): invalid memory type\n", __func__);
1278-
return -EINVAL;
1306+
ret = -EINVAL;
1307+
goto unlock;
12791308
}
12801309

12811310
if (vb->state != VB2_BUF_STATE_DEQUEUED) {
12821311
dprintk(1, "%s(): invalid buffer state %d\n", __func__, vb->state);
1283-
return -EINVAL;
1312+
ret = -EINVAL;
1313+
goto unlock;
12841314
}
12851315
ret = __verify_planes_array(vb, b);
12861316
if (ret < 0)
1287-
return ret;
1317+
goto unlock;
1318+
12881319
ret = __buf_prepare(vb, b);
12891320
if (ret < 0)
1290-
return ret;
1321+
goto unlock;
12911322

12921323
__fill_v4l2_buffer(vb, b);
12931324

1294-
return 0;
1325+
unlock:
1326+
if (mmap_sem)
1327+
up_read(mmap_sem);
1328+
return ret;
12951329
}
12961330
EXPORT_SYMBOL_GPL(vb2_prepare_buf);
12971331

0 commit comments

Comments
 (0)