[Patch 2/2 resend] Prevent vga16fb from accessing hw after it was unregistered

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux