diff options
author | Andrea Righi <righi.andrea@gmail.com> | 2009-02-04 18:12:03 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2009-02-05 15:56:46 -0500 |
commit | 1f5e31d7e55ac7fbd4ec5e5b20c8868b0e4564c9 (patch) | |
tree | 713d0ace63c95da9b989aafce8ec84ebb1d1cbc3 | |
parent | afd8d0f940ba5078f38e435440089117ac7d9eb4 (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.c | 20 | ||||
-rw-r--r-- | drivers/video/fbmem.c | 135 | ||||
-rw-r--r-- | include/linux/fb.h | 15 |
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); |
275 | out1: | ||
276 | unlock_fb_info(info); | ||
277 | out: | ||
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 | ||
1128 | static long fb_ioctl(struct file *file, unsigned int cmd, unsigned long arg) | 1142 | static 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 | ||
1258 | static long fb_compat_ioctl(struct file *file, unsigned int cmd, | 1265 | static 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]; | |||
960 | extern int num_registered_fb; | 960 | extern int num_registered_fb; |
961 | extern struct class *fb_class; | 961 | extern struct class *fb_class; |
962 | 962 | ||
963 | static 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 | |||
973 | static inline void unlock_fb_info(struct fb_info *info) | ||
974 | { | ||
975 | mutex_unlock(&info->lock); | ||
976 | } | ||
977 | |||
963 | static inline void __fb_pad_aligned_buffer(u8 *dst, u32 d_pitch, | 978 | static 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 | { |