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>
diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c
index 91b78e6..f53b9f1 100644
--- a/drivers/video/fbcmap.c
+++ b/drivers/video/fbcmap.c
@@ -250,10 +250,6 @@
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 @@
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;
}