From 9900132f3437e9373aa030cdb5bd2d5db15566e3 Mon Sep 17 00:00:00 2001 From: Maxim Levitsky Date: Thu, 27 Sep 2007 20:34:09 -0300 Subject: [PATCH] V4L/DVB (6268): V4L: Fix a lock inversion in generic videobuf code 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 http://thread.gmane.org/gmane.comp.video.video4linux/34978/focus=34981 Reviewed-by: Ricardo Cerqueira Signed-off-by: Mauro Carvalho Chehab --- drivers/media/video/videobuf-core.c | 7 ++++++ drivers/media/video/videobuf-dma-sg.c | 32 ++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c index 3bd06bb633a7..a27e114cacef 100644 --- a/drivers/media/video/videobuf-core.c +++ b/drivers/media/video/videobuf-core.c @@ -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(¤t->mm->mmap_sem); + mutex_lock(&q->lock); retval = -EBUSY; if (q->reading) { @@ -434,6 +437,10 @@ int videobuf_qbuf(struct videobuf_queue *q, done: mutex_unlock(&q->lock); + + if (b->memory == V4L2_MEMORY_MMAP) + up_read(¤t->mm->mmap_sem); + return retval; } diff --git a/drivers/media/video/videobuf-dma-sg.c b/drivers/media/video/videobuf-dma-sg.c index 0939ede831ab..05dd38343fa3 100644 --- a/drivers/media/video/videobuf-dma-sg.c +++ b/drivers/media/video/videobuf-dma-sg.c @@ -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; @@ -160,12 +160,12 @@ int videobuf_dma_init_user(struct videobuf_dmabuf *dma, int direction, dma->varea = (void *) data; - down_read(¤t->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(¤t->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); @@ -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(¤t->mm->mmap_sem); + ret = videobuf_dma_init_user_locked(dma, direction, data, size); + up_read(¤t->mm->mmap_sem); + + return ret; +} + int videobuf_dma_init_kernel(struct videobuf_dmabuf *dma, int direction, int nr_pages) { @@ -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: -- 2.20.1