fbmem: don't call copy_from/to_user() with mutex held

Avoid calling copy_from/to_user() with fb_info->lock mutex held in fbmem
ioctl().

fb_mmap() is called under mm->mmap_sem (A) held, that also acquires
fb_info->lock (B); fb_ioctl() takes fb_info->lock (B) and does
copy_from/to_user() that might acquire mm->mmap_sem (A), causing a
deadlock.

NOTE: it doesn't push down the fb_info->lock in each own driver's
fb_ioctl(), so there are still potential deadlocks elsewhere.

Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
Cc: Dave Jones <davej@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Johannes Weiner <hannes@saeurebad.de>
Cc: Krzysztof Helt <krzysztof.h1@wp.pl>
Cc: Harvey Harrison <harvey.harrison@gmail.com>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

1 files changed, 14 insertions, 6 deletions

diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c
index 91b78e691505..f53b9f1d6aba 100644
--- a/drivers/video/fbcmap.c
+++ b/drivers/video/fbcmap.c
@@ -250,10 +250,6 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info)
         int rc, size = cmap->len * sizeof(u16);
         struct fb_cmap umap;
 
-        if (cmap->start < 0 || (!info->fbops->fb_setcolreg &&
-                                !info->fbops->fb_setcmap))
-                return -EINVAL;
-
         memset(&umap, 0, sizeof(struct fb_cmap));
         rc = fb_alloc_cmap(&umap, cmap->len, cmap->transp != NULL);
         if (rc)
@@ -262,11 +258,23 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info)
             copy_from_user(umap.green, cmap->green, size) ||
             copy_from_user(umap.blue, cmap->blue, size) ||
             (cmap->transp && copy_from_user(umap.transp, cmap->transp, size))) {
-                fb_dealloc_cmap(&umap);
-                return -EFAULT;
+                rc = -EFAULT;
+                goto out;
         }
         umap.start = cmap->start;
+        if (!lock_fb_info(info)) {
+                rc = -ENODEV;
+                goto out;
+        }
+        if (cmap->start < 0 || (!info->fbops->fb_setcolreg &&
+                                !info->fbops->fb_setcmap)) {
+                rc = -EINVAL;
+                goto out1;
+        }
         rc = fb_set_cmap(&umap, info);
+out1:
+        unlock_fb_info(info);
+out:
         fb_dealloc_cmap(&umap);
         return rc;
 }