Commit 698b3682 authored by Linus Torvalds's avatar Linus Torvalds

fbcon: add lifetime refcount to opened frame buffers

This just adds the refcount and the new registration lock logic.  It
does not (for example) actually change the read/write/ioctl routines to
actually use the frame buffer that was opened: those function still end
up alway susing whatever the current frame buffer is at the time of the
call.

Without this, if something holds the frame buffer open over a
framebuffer switch, the close() operation after the switch will access a
fb_info that has been free'd by the unregistering of the old frame
buffer.

(The read/write/ioctl operations will normally not cause problems,
because they will - illogically - pick up the new fbcon instead.  But a
switch that happens just as one of those is going on might see problems
too, the window is just much smaller: one individual op rather than the
whole open-close sequence.)

This use-after-free is apparently fairly easily triggered by the Ubuntu
11.04 boot sequence.
Acked-by: default avatarTim Gardner <tim.gardner@canonical.com>
Tested-by: default avatarDaniel J Blueman <daniel.blueman@gmail.com>
Tested-by: default avatarAnca Emanuel <anca.emanuel@gmail.com>
Cc: Bruno Prémont <bonbons@linux-vserver.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Andy Whitcroft <andy.whitcroft@canonical.com>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 9f381a61
...@@ -42,9 +42,34 @@ ...@@ -42,9 +42,34 @@
#define FBPIXMAPSIZE (1024 * 8) #define FBPIXMAPSIZE (1024 * 8)
static DEFINE_MUTEX(registration_lock);
struct fb_info *registered_fb[FB_MAX] __read_mostly; struct fb_info *registered_fb[FB_MAX] __read_mostly;
int num_registered_fb __read_mostly; int num_registered_fb __read_mostly;
static struct fb_info *get_fb_info(unsigned int idx)
{
struct fb_info *fb_info;
if (idx >= FB_MAX)
return ERR_PTR(-ENODEV);
mutex_lock(&registration_lock);
fb_info = registered_fb[idx];
if (fb_info)
atomic_inc(&fb_info->count);
mutex_unlock(&registration_lock);
return fb_info;
}
static void put_fb_info(struct fb_info *fb_info)
{
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
int lock_fb_info(struct fb_info *info) int lock_fb_info(struct fb_info *info)
{ {
mutex_lock(&info->lock); mutex_lock(&info->lock);
...@@ -647,6 +672,7 @@ int fb_show_logo(struct fb_info *info, int rotate) { return 0; } ...@@ -647,6 +672,7 @@ int fb_show_logo(struct fb_info *info, int rotate) { return 0; }
static void *fb_seq_start(struct seq_file *m, loff_t *pos) static void *fb_seq_start(struct seq_file *m, loff_t *pos)
{ {
mutex_lock(&registration_lock);
return (*pos < FB_MAX) ? pos : NULL; return (*pos < FB_MAX) ? pos : NULL;
} }
...@@ -658,6 +684,7 @@ static void *fb_seq_next(struct seq_file *m, void *v, loff_t *pos) ...@@ -658,6 +684,7 @@ static void *fb_seq_next(struct seq_file *m, void *v, loff_t *pos)
static void fb_seq_stop(struct seq_file *m, void *v) static void fb_seq_stop(struct seq_file *m, void *v)
{ {
mutex_unlock(&registration_lock);
} }
static int fb_seq_show(struct seq_file *m, void *v) static int fb_seq_show(struct seq_file *m, void *v)
...@@ -1361,14 +1388,16 @@ __releases(&info->lock) ...@@ -1361,14 +1388,16 @@ __releases(&info->lock)
struct fb_info *info; struct fb_info *info;
int res = 0; int res = 0;
if (fbidx >= FB_MAX) info = get_fb_info(fbidx);
return -ENODEV; if (!info) {
info = registered_fb[fbidx];
if (!info)
request_module("fb%d", fbidx); request_module("fb%d", fbidx);
info = registered_fb[fbidx]; info = get_fb_info(fbidx);
if (!info) if (!info)
return -ENODEV; return -ENODEV;
}
if (IS_ERR(info))
return PTR_ERR(info);
mutex_lock(&info->lock); mutex_lock(&info->lock);
if (!try_module_get(info->fbops->owner)) { if (!try_module_get(info->fbops->owner)) {
res = -ENODEV; res = -ENODEV;
...@@ -1386,6 +1415,8 @@ __releases(&info->lock) ...@@ -1386,6 +1415,8 @@ __releases(&info->lock)
#endif #endif
out: out:
mutex_unlock(&info->lock); mutex_unlock(&info->lock);
if (res)
put_fb_info(info);
return res; return res;
} }
...@@ -1401,6 +1432,7 @@ __releases(&info->lock) ...@@ -1401,6 +1432,7 @@ __releases(&info->lock)
info->fbops->fb_release(info,1); info->fbops->fb_release(info,1);
module_put(info->fbops->owner); module_put(info->fbops->owner);
mutex_unlock(&info->lock); mutex_unlock(&info->lock);
put_fb_info(info);
return 0; return 0;
} }
...@@ -1542,11 +1574,13 @@ register_framebuffer(struct fb_info *fb_info) ...@@ -1542,11 +1574,13 @@ register_framebuffer(struct fb_info *fb_info)
remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id, remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id,
fb_is_primary_device(fb_info)); fb_is_primary_device(fb_info));
mutex_lock(&registration_lock);
num_registered_fb++; num_registered_fb++;
for (i = 0 ; i < FB_MAX; i++) for (i = 0 ; i < FB_MAX; i++)
if (!registered_fb[i]) if (!registered_fb[i])
break; break;
fb_info->node = i; fb_info->node = i;
atomic_set(&fb_info->count, 1);
mutex_init(&fb_info->lock); mutex_init(&fb_info->lock);
mutex_init(&fb_info->mm_lock); mutex_init(&fb_info->mm_lock);
...@@ -1583,6 +1617,7 @@ register_framebuffer(struct fb_info *fb_info) ...@@ -1583,6 +1617,7 @@ register_framebuffer(struct fb_info *fb_info)
fb_var_to_videomode(&mode, &fb_info->var); fb_var_to_videomode(&mode, &fb_info->var);
fb_add_videomode(&mode, &fb_info->modelist); fb_add_videomode(&mode, &fb_info->modelist);
registered_fb[i] = fb_info; registered_fb[i] = fb_info;
mutex_unlock(&registration_lock);
event.info = fb_info; event.info = fb_info;
if (!lock_fb_info(fb_info)) if (!lock_fb_info(fb_info))
...@@ -1616,6 +1651,7 @@ unregister_framebuffer(struct fb_info *fb_info) ...@@ -1616,6 +1651,7 @@ unregister_framebuffer(struct fb_info *fb_info)
struct fb_event event; struct fb_event event;
int i, ret = 0; int i, ret = 0;
mutex_lock(&registration_lock);
i = fb_info->node; i = fb_info->node;
if (!registered_fb[i]) { if (!registered_fb[i]) {
ret = -EINVAL; ret = -EINVAL;
...@@ -1638,7 +1674,7 @@ unregister_framebuffer(struct fb_info *fb_info) ...@@ -1638,7 +1674,7 @@ unregister_framebuffer(struct fb_info *fb_info)
(fb_info->pixmap.flags & FB_PIXMAP_DEFAULT)) (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT))
kfree(fb_info->pixmap.addr); kfree(fb_info->pixmap.addr);
fb_destroy_modelist(&fb_info->modelist); fb_destroy_modelist(&fb_info->modelist);
registered_fb[i]=NULL; registered_fb[i] = NULL;
num_registered_fb--; num_registered_fb--;
fb_cleanup_device(fb_info); fb_cleanup_device(fb_info);
device_destroy(fb_class, MKDEV(FB_MAJOR, i)); device_destroy(fb_class, MKDEV(FB_MAJOR, i));
...@@ -1646,9 +1682,9 @@ unregister_framebuffer(struct fb_info *fb_info) ...@@ -1646,9 +1682,9 @@ unregister_framebuffer(struct fb_info *fb_info)
fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event); fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
/* this may free fb info */ /* this may free fb info */
if (fb_info->fbops->fb_destroy) put_fb_info(fb_info);
fb_info->fbops->fb_destroy(fb_info);
done: done:
mutex_unlock(&registration_lock);
return ret; return ret;
} }
......
...@@ -832,6 +832,7 @@ struct fb_tile_ops { ...@@ -832,6 +832,7 @@ struct fb_tile_ops {
#define FBINFO_CAN_FORCE_OUTPUT 0x200000 #define FBINFO_CAN_FORCE_OUTPUT 0x200000
struct fb_info { struct fb_info {
atomic_t count;
int node; int node;
int flags; int flags;
struct mutex lock; /* Lock for open/release/ioctl funcs */ struct mutex lock; /* Lock for open/release/ioctl funcs */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment