Skip to content

Commit

Permalink
V4L/DVB (6268): V4L: Fix a lock inversion in generic videobuf code
Browse files Browse the repository at this point in the history
videobuf_qbuf takes q->lock, and then calls
q->ops->buf_prepare which by design in all drivers calls
videobuf_iolock which calls videobuf_dma_init_user and this
takes current->mm->mmap_sem

on the other hand if user calls mumap from other thread, sys_munmap
takes current->mm->mmap_sem and videobuf_vm_close takes q->lock

Since this can occur only for V4L2_MEMORY_MMAP buffers, take
current->mm->mmap_sem in qbuf, before q->lock, and don't take
current->mm->mmap_sem videobuf_dma_init_user for those buffers

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
http://thread.gmane.org/gmane.comp.video.video4linux/34978/focus=34981
Reviewed-by: Ricardo Cerqueira <v4l@cerqueira.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>
  • Loading branch information
Maxim Levitsky authored and Mauro Carvalho Chehab committed Oct 10, 2007
1 parent 851c0c9 commit 9900132
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 5 deletions.
7 changes: 7 additions & 0 deletions drivers/media/video/videobuf-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,9 @@ int videobuf_qbuf(struct videobuf_queue *q,

MAGIC_CHECK(q->int_ops->magic,MAGIC_QTYPE_OPS);

if (b->memory == V4L2_MEMORY_MMAP)
down_read(&current->mm->mmap_sem);

mutex_lock(&q->lock);
retval = -EBUSY;
if (q->reading) {
Expand Down Expand Up @@ -434,6 +437,10 @@ int videobuf_qbuf(struct videobuf_queue *q,

done:
mutex_unlock(&q->lock);

if (b->memory == V4L2_MEMORY_MMAP)
up_read(&current->mm->mmap_sem);

return retval;
}

Expand Down
32 changes: 27 additions & 5 deletions drivers/media/video/videobuf-dma-sg.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ void videobuf_dma_init(struct videobuf_dmabuf *dma)
dma->magic = MAGIC_DMABUF;
}

int videobuf_dma_init_user(struct videobuf_dmabuf *dma, int direction,
unsigned long data, unsigned long size)
static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
int direction, unsigned long data, unsigned long size)
{
unsigned long first,last;
int err, rw = 0;
Expand All @@ -160,12 +160,12 @@ int videobuf_dma_init_user(struct videobuf_dmabuf *dma, int direction,

dma->varea = (void *) data;

down_read(&current->mm->mmap_sem);

err = get_user_pages(current,current->mm,
data & PAGE_MASK, dma->nr_pages,
rw == READ, 1, /* force */
dma->pages, NULL);
up_read(&current->mm->mmap_sem);

if (err != dma->nr_pages) {
dma->nr_pages = (err >= 0) ? err : 0;
dprintk(1,"get_user_pages: err=%d [%d]\n",err,dma->nr_pages);
Expand All @@ -174,6 +174,17 @@ int videobuf_dma_init_user(struct videobuf_dmabuf *dma, int direction,
return 0;
}

int videobuf_dma_init_user(struct videobuf_dmabuf *dma, int direction,
unsigned long data, unsigned long size)
{
int ret;
down_read(&current->mm->mmap_sem);
ret = videobuf_dma_init_user_locked(dma, direction, data, size);
up_read(&current->mm->mmap_sem);

return ret;
}

int videobuf_dma_init_kernel(struct videobuf_dmabuf *dma, int direction,
int nr_pages)
{
Expand Down Expand Up @@ -469,13 +480,24 @@ static int __videobuf_iolock (struct videobuf_queue* q,
pages );
if (0 != err)
return err;
} else {
} else if (vb->memory == V4L2_MEMORY_USERPTR) {
/* dma directly to userspace */
err = videobuf_dma_init_user( &mem->dma,
PCI_DMA_FROMDEVICE,
vb->baddr,vb->bsize );
if (0 != err)
return err;
} else {
/* NOTE: HACK: videobuf_iolock on V4L2_MEMORY_MMAP
buffers can only be called from videobuf_qbuf
we take current->mm->mmap_sem there, to prevent
locking inversion, so don't take it here */

err = videobuf_dma_init_user_locked(&mem->dma,
PCI_DMA_FROMDEVICE,
vb->baddr, vb->bsize);
if (0 != err)
return err;
}
break;
case V4L2_MEMORY_OVERLAY:
Expand Down

0 comments on commit 9900132

Please sign in to comment.