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 <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>

diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c
index 3bd06bb633a717ecdb39b28a0a7f2569c3f1c2d5..a27e114cacef94dc2526d628e49b9236fa564cf4 100644 (file)
--- 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(&current->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(&current->mm->mmap_sem);
+
        return retval;
 }
 

diff --git a/drivers/media/video/videobuf-dma-sg.c b/drivers/media/video/videobuf-dma-sg.c
index 0939ede831ab3fac8c36e3e746abbec27e12f450..05dd38343fa36171d187b50979380ffcb2640b80 100644 (file)
--- 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(&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);
@@ -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)
 {
@@ -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: