aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrea Righi <righi.andrea@gmail.com>2009-02-04 18:12:03 -0500
committerLinus Torvalds <torvalds@linux-foundation.org>2009-02-05 15:56:46 -0500
commit1f5e31d7e55ac7fbd4ec5e5b20c8868b0e4564c9 (patch)
tree713d0ace63c95da9b989aafce8ec84ebb1d1cbc3
parentafd8d0f940ba5078f38e435440089117ac7d9eb4 (diff)
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>
-rw-r--r--drivers/video/fbcmap.c20
-rw-r--r--drivers/video/fbmem.c135
-rw-r--r--include/linux/fb.h15
3 files changed, 98 insertions, 72 deletions
diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c
index 91b78e69150..f53b9f1d6ab 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)
250 int rc, size = cmap->len * sizeof(u16); 250 int rc, size = cmap->len * sizeof(u16);
251 struct fb_cmap umap; 251 struct fb_cmap umap;
252 252
253 if (cmap->start < 0 || (!info->fbops->fb_setcolreg &&
254 !info->fbops->fb_setcmap))
255 return -EINVAL;
256
257 memset(&umap, 0, sizeof(struct fb_cmap)); 253 memset(&umap, 0, sizeof(struct fb_cmap));
258 rc = fb_alloc_cmap(&umap, cmap->len, cmap->transp != NULL); 254 rc = fb_alloc_cmap(&umap, cmap->len, cmap->transp != NULL);
259 if (rc) 255 if (rc)
@@ -262,11 +258,23 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info)
262 copy_from_user(umap.green, cmap->green, size) || 258 copy_from_user(umap.green, cmap->green, size) ||
263 copy_from_user(umap.blue, cmap->blue, size) || 259 copy_from_user(umap.blue, cmap->blue, size) ||
264 (cmap->transp && copy_from_user(umap.transp, cmap->transp, size))) { 260 (cmap->transp && copy_from_user(umap.transp, cmap->transp, size))) {
265 fb_dealloc_cmap(&umap); 261 rc = -EFAULT;
266 return -EFAULT; 262 goto out;
267 } 263 }
268 umap.start = cmap->start; 264 umap.start = cmap->start;
265 if (!lock_fb_info(info)) {
266 rc = -ENODEV;
267 goto out;
268 }
269 if (cmap->start < 0 || (!info->fbops->fb_setcolreg &&
270 !info->fbops->fb_setcmap)) {
271 rc = -EINVAL;
272 goto out1;
273 }
269 rc = fb_set_cmap(&umap, info); 274 rc = fb_set_cmap(&umap, info);
275out1:
276 unlock_fb_info(info);
277out:
270 fb_dealloc_cmap(&umap); 278 fb_dealloc_cmap(&umap);
271 return rc; 279 return rc;
272} 280}
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 756efeb91ab..cfd9dce1ce0 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1013,132 +1013,139 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
1013 struct fb_var_screeninfo var; 1013 struct fb_var_screeninfo var;
1014 struct fb_fix_screeninfo fix; 1014 struct fb_fix_screeninfo fix;
1015 struct fb_con2fbmap con2fb; 1015 struct fb_con2fbmap con2fb;
1016 struct fb_cmap cmap_from;
1016 struct fb_cmap_user cmap; 1017 struct fb_cmap_user cmap;
1017 struct fb_event event; 1018 struct fb_event event;
1018 void __user *argp = (void __user *)arg; 1019 void __user *argp = (void __user *)arg;
1019 long ret = 0; 1020 long ret = 0;
1020 1021
1021 fb = info->fbops;
1022 if (!fb)
1023 return -ENODEV;
1024
1025 switch (cmd) { 1022 switch (cmd) {
1026 case FBIOGET_VSCREENINFO: 1023 case FBIOGET_VSCREENINFO:
1027 ret = copy_to_user(argp, &info->var, 1024 if (!lock_fb_info(info))
1028 sizeof(var)) ? -EFAULT : 0; 1025 return -ENODEV;
1026 var = info->var;
1027 unlock_fb_info(info);
1028
1029 ret = copy_to_user(argp, &var, sizeof(var)) ? -EFAULT : 0;
1029 break; 1030 break;
1030 case FBIOPUT_VSCREENINFO: 1031 case FBIOPUT_VSCREENINFO:
1031 if (copy_from_user(&var, argp, sizeof(var))) { 1032 if (copy_from_user(&var, argp, sizeof(var)))
1032 ret = -EFAULT; 1033 return -EFAULT;
1033 break; 1034 if (!lock_fb_info(info))
1034 } 1035 return -ENODEV;
1035 acquire_console_sem(); 1036 acquire_console_sem();
1036 info->flags |= FBINFO_MISC_USEREVENT; 1037 info->flags |= FBINFO_MISC_USEREVENT;
1037 ret = fb_set_var(info, &var); 1038 ret = fb_set_var(info, &var);
1038 info->flags &= ~FBINFO_MISC_USEREVENT; 1039 info->flags &= ~FBINFO_MISC_USEREVENT;
1039 release_console_sem(); 1040 release_console_sem();
1040 if (ret == 0 && copy_to_user(argp, &var, sizeof(var))) 1041 unlock_fb_info(info);
1042 if (!ret && copy_to_user(argp, &var, sizeof(var)))
1041 ret = -EFAULT; 1043 ret = -EFAULT;
1042 break; 1044 break;
1043 case FBIOGET_FSCREENINFO: 1045 case FBIOGET_FSCREENINFO:
1044 ret = copy_to_user(argp, &info->fix, 1046 if (!lock_fb_info(info))
1045 sizeof(fix)) ? -EFAULT : 0; 1047 return -ENODEV;
1048 fix = info->fix;
1049 unlock_fb_info(info);
1050
1051 ret = copy_to_user(argp, &fix, sizeof(fix)) ? -EFAULT : 0;
1046 break; 1052 break;
1047 case FBIOPUTCMAP: 1053 case FBIOPUTCMAP:
1048 if (copy_from_user(&cmap, argp, sizeof(cmap))) 1054 if (copy_from_user(&cmap, argp, sizeof(cmap)))
1049 ret = -EFAULT; 1055 return -EFAULT;
1050 else 1056 ret = fb_set_user_cmap(&cmap, info);
1051 ret = fb_set_user_cmap(&cmap, info);
1052 break; 1057 break;
1053 case FBIOGETCMAP: 1058 case FBIOGETCMAP:
1054 if (copy_from_user(&cmap, argp, sizeof(cmap))) 1059 if (copy_from_user(&cmap, argp, sizeof(cmap)))
1055 ret = -EFAULT; 1060 return -EFAULT;
1056 else 1061 if (!lock_fb_info(info))
1057 ret = fb_cmap_to_user(&info->cmap, &cmap); 1062 return -ENODEV;
1063 cmap_from = info->cmap;
1064 unlock_fb_info(info);
1065 ret = fb_cmap_to_user(&cmap_from, &cmap);
1058 break; 1066 break;
1059 case FBIOPAN_DISPLAY: 1067 case FBIOPAN_DISPLAY:
1060 if (copy_from_user(&var, argp, sizeof(var))) { 1068 if (copy_from_user(&var, argp, sizeof(var)))
1061 ret = -EFAULT; 1069 return -EFAULT;
1062 break; 1070 if (!lock_fb_info(info))
1063 } 1071 return -ENODEV;
1064 acquire_console_sem(); 1072 acquire_console_sem();
1065 ret = fb_pan_display(info, &var); 1073 ret = fb_pan_display(info, &var);
1066 release_console_sem(); 1074 release_console_sem();
1075 unlock_fb_info(info);
1067 if (ret == 0 && copy_to_user(argp, &var, sizeof(var))) 1076 if (ret == 0 && copy_to_user(argp, &var, sizeof(var)))
1068 ret = -EFAULT; 1077 return -EFAULT;
1069 break; 1078 break;
1070 case FBIO_CURSOR: 1079 case FBIO_CURSOR:
1071 ret = -EINVAL; 1080 ret = -EINVAL;
1072 break; 1081 break;
1073 case FBIOGET_CON2FBMAP: 1082 case FBIOGET_CON2FBMAP:
1074 if (copy_from_user(&con2fb, argp, sizeof(con2fb))) 1083 if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
1075 ret = -EFAULT; 1084 return -EFAULT;
1076 else if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES) 1085 if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES)
1077 ret = -EINVAL; 1086 return -EINVAL;
1078 else { 1087 con2fb.framebuffer = -1;
1079 con2fb.framebuffer = -1; 1088 event.data = &con2fb;
1080 event.info = info; 1089
1081 event.data = &con2fb; 1090 if (!lock_fb_info(info))
1082 fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, 1091 return -ENODEV;
1083 &event); 1092 event.info = info;
1084 ret = copy_to_user(argp, &con2fb, 1093 fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, &event);
1085 sizeof(con2fb)) ? -EFAULT : 0; 1094 unlock_fb_info(info);
1086 } 1095
1096 ret = copy_to_user(argp, &con2fb, sizeof(con2fb)) ? -EFAULT : 0;
1087 break; 1097 break;
1088 case FBIOPUT_CON2FBMAP: 1098 case FBIOPUT_CON2FBMAP:
1089 if (copy_from_user(&con2fb, argp, sizeof(con2fb))) { 1099 if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
1090 ret = -EFAULT; 1100 return -EFAULT;
1091 break; 1101 if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES)
1092 } 1102 return -EINVAL;
1093 if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES) { 1103 if (con2fb.framebuffer < 0 || con2fb.framebuffer >= FB_MAX)
1094 ret = -EINVAL; 1104 return -EINVAL;
1095 break;
1096 }
1097 if (con2fb.framebuffer < 0 || con2fb.framebuffer >= FB_MAX) {
1098 ret = -EINVAL;
1099 break;
1100 }
1101 if (!registered_fb[con2fb.framebuffer]) 1105 if (!registered_fb[con2fb.framebuffer])
1102 request_module("fb%d", con2fb.framebuffer); 1106 request_module("fb%d", con2fb.framebuffer);
1103 if (!registered_fb[con2fb.framebuffer]) { 1107 if (!registered_fb[con2fb.framebuffer]) {
1104 ret = -EINVAL; 1108 ret = -EINVAL;
1105 break; 1109 break;
1106 } 1110 }
1107 event.info = info;
1108 event.data = &con2fb; 1111 event.data = &con2fb;
1112 if (!lock_fb_info(info))
1113 return -ENODEV;
1114 event.info = info;
1109 ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, 1115 ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP,
1110 &event); 1116 &event);
1117 unlock_fb_info(info);
1111 break; 1118 break;
1112 case FBIOBLANK: 1119 case FBIOBLANK:
1120 if (!lock_fb_info(info))
1121 return -ENODEV;
1113 acquire_console_sem(); 1122 acquire_console_sem();
1114 info->flags |= FBINFO_MISC_USEREVENT; 1123 info->flags |= FBINFO_MISC_USEREVENT;
1115 ret = fb_blank(info, arg); 1124 ret = fb_blank(info, arg);
1116 info->flags &= ~FBINFO_MISC_USEREVENT; 1125 info->flags &= ~FBINFO_MISC_USEREVENT;
1117 release_console_sem(); 1126 release_console_sem();
1118 break;; 1127 unlock_fb_info(info);
1128 break;
1119 default: 1129 default:
1120 if (fb->fb_ioctl == NULL) 1130 if (!lock_fb_info(info))
1121 ret = -ENOTTY; 1131 return -ENODEV;
1122 else 1132 fb = info->fbops;
1133 if (fb->fb_ioctl)
1123 ret = fb->fb_ioctl(info, cmd, arg); 1134 ret = fb->fb_ioctl(info, cmd, arg);
1135 else
1136 ret = -ENOTTY;
1137 unlock_fb_info(info);
1124 } 1138 }
1125 return ret; 1139 return ret;
1126} 1140}
1127 1141
1128static long fb_ioctl(struct file *file, unsigned int cmd, unsigned long arg) 1142static long fb_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
1129__acquires(&info->lock)
1130__releases(&info->lock)
1131{ 1143{
1132 struct inode *inode = file->f_path.dentry->d_inode; 1144 struct inode *inode = file->f_path.dentry->d_inode;
1133 int fbidx = iminor(inode); 1145 int fbidx = iminor(inode);
1134 struct fb_info *info; 1146 struct fb_info *info = registered_fb[fbidx];
1135 long ret;
1136 1147
1137 info = registered_fb[fbidx]; 1148 return do_fb_ioctl(info, cmd, arg);
1138 mutex_lock(&info->lock);
1139 ret = do_fb_ioctl(info, cmd, arg);
1140 mutex_unlock(&info->lock);
1141 return ret;
1142} 1149}
1143 1150
1144#ifdef CONFIG_COMPAT 1151#ifdef CONFIG_COMPAT
@@ -1257,8 +1264,6 @@ static int fb_get_fscreeninfo(struct fb_info *info, unsigned int cmd,
1257 1264
1258static long fb_compat_ioctl(struct file *file, unsigned int cmd, 1265static long fb_compat_ioctl(struct file *file, unsigned int cmd,
1259 unsigned long arg) 1266 unsigned long arg)
1260__acquires(&info->lock)
1261__releases(&info->lock)
1262{ 1267{
1263 struct inode *inode = file->f_path.dentry->d_inode; 1268 struct inode *inode = file->f_path.dentry->d_inode;
1264 int fbidx = iminor(inode); 1269 int fbidx = iminor(inode);
@@ -1266,7 +1271,6 @@ __releases(&info->lock)
1266 struct fb_ops *fb = info->fbops; 1271 struct fb_ops *fb = info->fbops;
1267 long ret = -ENOIOCTLCMD; 1272 long ret = -ENOIOCTLCMD;
1268 1273
1269 mutex_lock(&info->lock);
1270 switch(cmd) { 1274 switch(cmd) {
1271 case FBIOGET_VSCREENINFO: 1275 case FBIOGET_VSCREENINFO:
1272 case FBIOPUT_VSCREENINFO: 1276 case FBIOPUT_VSCREENINFO:
@@ -1292,7 +1296,6 @@ __releases(&info->lock)
1292 ret = fb->fb_compat_ioctl(info, cmd, arg); 1296 ret = fb->fb_compat_ioctl(info, cmd, arg);
1293 break; 1297 break;
1294 } 1298 }
1295 mutex_unlock(&info->lock);
1296 return ret; 1299 return ret;
1297} 1300}
1298#endif 1301#endif
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 818fe21257e..31527e17076 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -960,6 +960,21 @@ extern struct fb_info *registered_fb[FB_MAX];
960extern int num_registered_fb; 960extern int num_registered_fb;
961extern struct class *fb_class; 961extern struct class *fb_class;
962 962
963static inline int lock_fb_info(struct fb_info *info)
964{
965 mutex_lock(&info->lock);
966 if (!info->fbops) {
967 mutex_unlock(&info->lock);
968 return 0;
969 }
970 return 1;
971}
972
973static inline void unlock_fb_info(struct fb_info *info)
974{
975 mutex_unlock(&info->lock);
976}
977
963static inline void __fb_pad_aligned_buffer(u8 *dst, u32 d_pitch, 978static inline void __fb_pad_aligned_buffer(u8 *dst, u32 d_pitch,
964 u8 *src, u32 s_pitch, u32 height) 979 u8 *src, u32 s_pitch, u32 height)
965{ 980{