Since fb_info is refcounted it can successfully remain open while being unregistered (though not freed). Prevent the following sequence to corrupt modify hardware state behind back of native driver that took over control of hardware: - vga16fb registered + open /dev/fb0 (vga16fb) - load native driver - unregisters conflicting vga16fb - configures GPU with nouveaufb and more - fbcon takes over + close /dev/fb0 + vga16fb resets hw to vgacon mode (in fbops.fb_release) Add a new callback fb_unregistered to struct fbops to be called at end of unregister_framebuffer() so interested drivers can get know their fb was unregistered (previously they would know through fb_destroy callback but now that callback may happen way later) Prevent such hardware access by setting a flag with new fb_unregistered callback and checking it when vga16fb framebuffer is last closed and skipping vga state restore in case it's not registered anymore. CC: stable@xxxxxxxxxx Signed-off-by: Bruno PrÃmont <bonbons@xxxxxxxxxxxxxxxxx> --- Resend as my MUA decided to take wrong source address... This should also go into 2.6.39 stable as it didn't make it into 2.6.39 with the rest of fb_info refcounting work. This comes from [2.6.39-rc2, framebuffer] use after free oops ... [PATCH 0/2] fbcon sanity thread --- diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c index 5aac00e..bd9f93b 100644 --- a/drivers/video/fbmem.c +++ b/drivers/video/fbmem.c @@ -1661,6 +1661,11 @@ static int do_unregister_framebuffer(struct fb_info *fb_info) device_destroy(fb_class, MKDEV(FB_MAJOR, i)); event.info = fb_info; fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event); + if (fb_info->fbops->fb_unregistered) { + mutex_lock(&fb_info->lock); + fb_info->fbops->fb_unregistered(fb_info); + mutex_unlock(&fb_info->lock); + } /* this may free fb info */ put_fb_info(fb_info); diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c index 53b2c5a..dad96e1 100644 --- a/drivers/video/vga16fb.c +++ b/drivers/video/vga16fb.c @@ -59,7 +59,7 @@ struct vga16fb_par { struct vgastate state; unsigned int ref_count; int palette_blanked, vesa_blanked, mode, isVGA; - u8 misc, pel_msk, vss, clkdiv; + u8 misc, pel_msk, vss, clkdiv, unregistered; u8 crtc[VGA_CRT_C]; }; @@ -302,7 +302,7 @@ static int vga16fb_release(struct fb_info *info, int user) if (!par->ref_count) return -EINVAL; - if (par->ref_count == 1) + if (par->ref_count == 1 && !par->unregistered) restore_vga(&par->state); par->ref_count--; @@ -1271,19 +1271,26 @@ static void vga16fb_destroy(struct fb_info *info) framebuffer_release(info); } +static void vga16fb_unregistered(struct fb_info *info) +{ + struct vga16fb_par *par = info->par; + par->unregistered = 1; +} + static struct fb_ops vga16fb_ops = { - .owner = THIS_MODULE, - .fb_open = vga16fb_open, - .fb_release = vga16fb_release, - .fb_destroy = vga16fb_destroy, - .fb_check_var = vga16fb_check_var, - .fb_set_par = vga16fb_set_par, - .fb_setcolreg = vga16fb_setcolreg, - .fb_pan_display = vga16fb_pan_display, - .fb_blank = vga16fb_blank, - .fb_fillrect = vga16fb_fillrect, - .fb_copyarea = vga16fb_copyarea, - .fb_imageblit = vga16fb_imageblit, + .owner = THIS_MODULE, + .fb_open = vga16fb_open, + .fb_release = vga16fb_release, + .fb_destroy = vga16fb_destroy, + .fb_unregistered = vga16fb_unregistered, + .fb_check_var = vga16fb_check_var, + .fb_set_par = vga16fb_set_par, + .fb_setcolreg = vga16fb_setcolreg, + .fb_pan_display = vga16fb_pan_display, + .fb_blank = vga16fb_blank, + .fb_fillrect = vga16fb_fillrect, + .fb_copyarea = vga16fb_copyarea, + .fb_imageblit = vga16fb_imageblit, }; #ifndef MODULE diff --git a/include/linux/fb.h b/include/linux/fb.h index 6a82748..22a8796 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -685,6 +685,9 @@ struct fb_ops { void (*fb_get_caps)(struct fb_info *info, struct fb_blit_caps *caps, struct fb_var_screeninfo *var); + /* teardown hardware for this framebuffer (another driver may get + * ownership of hardware after this returns) */ + void (*fb_unregistered)(struct fb_info *info); /* teardown any resources to do with this framebuffer */ void (*fb_destroy)(struct fb_info *info); -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html