Patchwork [1/2] UBUNTU: SAUCE: fbcon -- fix race between open and removal of framebuffers

login
register
mail settings
Submitter Andy Whitcroft
Date July 29, 2010, 3:48 p.m.
Message ID <1280418501-9219-2-git-send-email-apw@canonical.com>
Download mbox | patch
Permalink /patch/60274/
State Accepted
Delegated to: Leann Ogasawara
Headers show

Comments

Andy Whitcroft - July 29, 2010, 3:48 p.m.
Currently there is no locking for updates to the registered_fb list.
This allows an open through /dev/fbN to pick up a registered framebuffer
pointer in parallel with it being released, as happens when a conflicting
framebuffer is ejected or on module unload.  There is also no reference
counting on the framebuffer descriptor which is referenced from all open
files, leading to references to released or reused memory to persist on
these open files.

This patch adds a reference count to the framebuffer descriptor to prevent
it from being released until after all pending opens are closed.  This
allows the pending opens to detect the closed status and unmap themselves.
It also adds locking to the framebuffer lookup path, locking it against
the removal path such that it is possible to atomically lookup and take a
reference to the descriptor.  It also adds locking to the read and write
paths which currently could access the framebuffer descriptor after it
has been freed.  Finally it moves the device to FBINFO_STATE_REMOVED to
indicate that all access should be errored for this device.

Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 drivers/video/fbmem.c |  132 ++++++++++++++++++++++++++++++++++++++-----------
 include/linux/fb.h    |    2 +
 2 files changed, 105 insertions(+), 29 deletions(-)
Stefan Bader - July 29, 2010, 3:56 p.m.
Apart from the odd thing to make err override cnt in one case and the other way
round in the other, which was there before the patch, this looks good.

Acked-by: Stefan Bader <stefan.bader@canonical.com>

On 07/29/2010 05:48 PM, Andy Whitcroft wrote:
> Currently there is no locking for updates to the registered_fb list.
> This allows an open through /dev/fbN to pick up a registered framebuffer
> pointer in parallel with it being released, as happens when a conflicting
> framebuffer is ejected or on module unload.  There is also no reference
> counting on the framebuffer descriptor which is referenced from all open
> files, leading to references to released or reused memory to persist on
> these open files.
> 
> This patch adds a reference count to the framebuffer descriptor to prevent
> it from being released until after all pending opens are closed.  This
> allows the pending opens to detect the closed status and unmap themselves.
> It also adds locking to the framebuffer lookup path, locking it against
> the removal path such that it is possible to atomically lookup and take a
> reference to the descriptor.  It also adds locking to the read and write
> paths which currently could access the framebuffer descriptor after it
> has been freed.  Finally it moves the device to FBINFO_STATE_REMOVED to
> indicate that all access should be errored for this device.
> 
> Signed-off-by: Andy Whitcroft <apw@canonical.com>
> ---
>  drivers/video/fbmem.c |  132 ++++++++++++++++++++++++++++++++++++++-----------
>  include/linux/fb.h    |    2 +
>  2 files changed, 105 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 731fce6..02eb135 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -42,6 +42,8 @@
>  
>  #define FBPIXMAPSIZE	(1024 * 8)
>  
> +/* Protects the registered framebuffer list and count. */
> +static DEFINE_SPINLOCK(registered_lock);
>  struct fb_info *registered_fb[FB_MAX] __read_mostly;
>  int num_registered_fb __read_mostly;
>  
> @@ -694,9 +696,7 @@ static ssize_t
>  fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>  {
>  	unsigned long p = *ppos;
> -	struct inode *inode = file->f_path.dentry->d_inode;
> -	int fbidx = iminor(inode);
> -	struct fb_info *info = registered_fb[fbidx];
> +	struct fb_info * const info = file->private_data;
>  	u32 *buffer, *dst;
>  	u32 __iomem *src;
>  	int c, i, cnt = 0, err = 0;
> @@ -705,19 +705,28 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>  	if (!info || ! info->screen_base)
>  		return -ENODEV;
>  
> -	if (info->state != FBINFO_STATE_RUNNING)
> -		return -EPERM;
> +	if (!lock_fb_info(info))
> +		return -ENODEV;
>  
> -	if (info->fbops->fb_read)
> -		return info->fbops->fb_read(info, buf, count, ppos);
> +	if (info->state != FBINFO_STATE_RUNNING) {
> +		err = -EPERM;
> +		goto out_fb_info;
> +	}
> +
> +	if (info->fbops->fb_read) {
> +		err = info->fbops->fb_read(info, buf, count, ppos);
> +		goto out_fb_info;
> +	}
>  	
>  	total_size = info->screen_size;
>  
>  	if (total_size == 0)
>  		total_size = info->fix.smem_len;
>  
> -	if (p >= total_size)
> -		return 0;
> +	if (p >= total_size) {
> +		err = 0;
> +		goto out_fb_info;
> +	}
>  
>  	if (count >= total_size)
>  		count = total_size;
> @@ -727,8 +736,10 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>  
>  	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
>  			 GFP_KERNEL);
> -	if (!buffer)
> -		return -ENOMEM;
> +	if (!buffer) {
> +		err = -ENOMEM;
> +		goto out_fb_info;
> +	}
>  
>  	src = (u32 __iomem *) (info->screen_base + p);
>  
> @@ -759,19 +770,21 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>  		cnt += c;
>  		count -= c;
>  	}
> +	if (!err)
> +		err = cnt;
>  
>  	kfree(buffer);
> +out_fb_info:
> +	unlock_fb_info(info);
>  
> -	return (err) ? err : cnt;
> +	return err;
>  }
>  
>  static ssize_t
>  fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>  {
>  	unsigned long p = *ppos;
> -	struct inode *inode = file->f_path.dentry->d_inode;
> -	int fbidx = iminor(inode);
> -	struct fb_info *info = registered_fb[fbidx];
> +	struct fb_info * const info = file->private_data;
>  	u32 *buffer, *src;
>  	u32 __iomem *dst;
>  	int c, i, cnt = 0, err = 0;
> @@ -780,8 +793,13 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>  	if (!info || !info->screen_base)
>  		return -ENODEV;
>  
> -	if (info->state != FBINFO_STATE_RUNNING)
> -		return -EPERM;
> +	if (!lock_fb_info(info))
> +		return -ENODEV;
> +
> +	if (info->state != FBINFO_STATE_RUNNING) {
> +		err = -EPERM;
> +		goto out_fb_info;
> +	}
>  
>  	if (info->fbops->fb_write)
>  		return info->fbops->fb_write(info, buf, count, ppos);
> @@ -791,8 +809,10 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>  	if (total_size == 0)
>  		total_size = info->fix.smem_len;
>  
> -	if (p > total_size)
> -		return -EFBIG;
> +	if (p > total_size) {
> +		err = -EFBIG;
> +		goto out_fb_info;
> +	}
>  
>  	if (count > total_size) {
>  		err = -EFBIG;
> @@ -808,8 +828,10 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>  
>  	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
>  			 GFP_KERNEL);
> -	if (!buffer)
> -		return -ENOMEM;
> +	if (!buffer) {
> +		err = -ENOMEM;
> +		goto out_fb_info;
> +	}
>  
>  	dst = (u32 __iomem *) (info->screen_base + p);
>  
> @@ -843,10 +865,14 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>  		cnt += c;
>  		count -= c;
>  	}
> +	if (cnt)
> +		err = cnt;
>  
>  	kfree(buffer);
> +out_fb_info:
> +	unlock_fb_info(info);
>  
> -	return (cnt) ? cnt : err;
> +	return err;
>  }
>  
>  int
> @@ -1321,8 +1347,7 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd,
>  static int
>  fb_mmap(struct file *file, struct vm_area_struct * vma)
>  {
> -	int fbidx = iminor(file->f_path.dentry->d_inode);
> -	struct fb_info *info = registered_fb[fbidx];
> +	struct fb_info * const info = file->private_data;
>  	struct fb_ops *fb = info->fbops;
>  	unsigned long off;
>  	unsigned long start;
> @@ -1334,6 +1359,11 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
>  	if (!fb)
>  		return -ENODEV;
>  	mutex_lock(&info->mm_lock);
> +	if (info->state == FBINFO_STATE_REMOVED) {
> +		mutex_unlock(&info->mm_lock);
> +		return -ENODEV;
> +	}
> +
>  	if (fb->fb_mmap) {
>  		int res;
>  		res = fb->fb_mmap(info, vma);
> @@ -1369,6 +1399,34 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
>  	return 0;
>  }
>  
> +static struct fb_info *get_framebuffer_info(int idx)
> +__acquires(&registered_lock)
> +__releases(&registered_lock)
> +{
> +	struct fb_info *fb_info;
> +
> +	spin_lock(&registered_lock);
> +	fb_info = registered_fb[idx];
> +	fb_info->ref_count++;
> +	spin_unlock(&registered_lock);
> +
> +	return fb_info;
> +}
> +
> +static void put_framebuffer_info(struct fb_info *fb_info)
> +__acquires(&registered_lock)
> +__releases(&registered_lock)
> +{
> +	int keep;
> +
> +	spin_lock(&registered_lock);
> +	keep = --fb_info->ref_count;
> +	spin_unlock(&registered_lock);
> +	
> +	if (!keep && fb_info->fbops->fb_destroy)
> +		fb_info->fbops->fb_destroy(fb_info);
> +}
> +
>  static int
>  fb_open(struct inode *inode, struct file *file)
>  __acquires(&info->lock)
> @@ -1380,13 +1438,17 @@ __releases(&info->lock)
>  
>  	if (fbidx >= FB_MAX)
>  		return -ENODEV;
> -	info = registered_fb[fbidx];
> +	info = get_framebuffer_info(fbidx);
>  	if (!info)
>  		request_module("fb%d", fbidx);
> -	info = registered_fb[fbidx];
> +	info = get_framebuffer_info(fbidx);
>  	if (!info)
>  		return -ENODEV;
>  	mutex_lock(&info->lock);
> +	if (info->state == FBINFO_STATE_REMOVED) {
> +		res = -ENODEV;
> +		goto out;
> +	}
>  	if (!try_module_get(info->fbops->owner)) {
>  		res = -ENODEV;
>  		goto out;
> @@ -1403,6 +1465,8 @@ __releases(&info->lock)
>  #endif
>  out:
>  	mutex_unlock(&info->lock);
> +	if (res)
> +		put_framebuffer_info(info);
>  	return res;
>  }
>  
> @@ -1418,6 +1482,7 @@ __releases(&info->lock)
>  		info->fbops->fb_release(info,1);
>  	module_put(info->fbops->owner);
>  	mutex_unlock(&info->lock);
> +	put_framebuffer_info(info);
>  	return 0;
>  }
>  
> @@ -1565,6 +1630,7 @@ register_framebuffer(struct fb_info *fb_info)
>  	fb_info->node = i;
>  	mutex_init(&fb_info->lock);
>  	mutex_init(&fb_info->mm_lock);
> +	fb_info->ref_count = 1;
>  
>  	fb_info->dev = device_create(fb_class, fb_info->device,
>  				     MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
> @@ -1608,7 +1674,6 @@ register_framebuffer(struct fb_info *fb_info)
>  	return 0;
>  }
>  
> -
>  /**
>   *	unregister_framebuffer - releases a frame buffer device
>   *	@fb_info: frame buffer info structure
> @@ -1643,6 +1708,16 @@ unregister_framebuffer(struct fb_info *fb_info)
>  		return -ENODEV;
>  	event.info = fb_info;
>  	ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
> +	if (!ret) {
> +		mutex_lock(&fb_info->mm_lock);
> +		/*
> +		 * We must prevent any operations for this transition, we
> +		 * already have info->lock so grab the info->mm_lock to hold
> +		 * the remainder.
> +		 */
> +		fb_info->state = FBINFO_STATE_REMOVED;
> +		mutex_unlock(&fb_info->mm_lock);
> +	}
>  	unlock_fb_info(fb_info);
>  
>  	if (ret) {
> @@ -1662,8 +1737,7 @@ unregister_framebuffer(struct fb_info *fb_info)
>  	fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
>  
>  	/* this may free fb info */
> -	if (fb_info->fbops->fb_destroy)
> -		fb_info->fbops->fb_destroy(fb_info);
> +	put_framebuffer_info(fb_info);
>  done:
>  	return ret;
>  }
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index e7445df..0119891 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -815,6 +815,7 @@ struct fb_tile_ops {
>  struct fb_info {
>  	int node;
>  	int flags;
> +	int ref_count;
>  	struct mutex lock;		/* Lock for open/release/ioctl funcs */
>  	struct mutex mm_lock;		/* Lock for fb_mmap and smem_* fields */
>  	struct fb_var_screeninfo var;	/* Current var */
> @@ -854,6 +855,7 @@ struct fb_info {
>  	void *pseudo_palette;		/* Fake palette of 16 colors */ 
>  #define FBINFO_STATE_RUNNING	0
>  #define FBINFO_STATE_SUSPENDED	1
> +#define FBINFO_STATE_REMOVED	2
>  	u32 state;			/* Hardware state i.e suspend */
>  	void *fbcon_par;                /* fbcon use-only private area */
>  	/* From here on everything is device dependent */
Andy Whitcroft - July 29, 2010, 4:09 p.m.
On Thu, Jul 29, 2010 at 05:56:46PM +0200, Stefan Bader wrote:
> Apart from the odd thing to make err override cnt in one case and the other way
> round in the other, which was there before the patch, this looks good.

Yeah I suspect that that is a bug, that the cnt should always override
any error.  But that as you say was there before.

> Acked-by: Stefan Bader <stefan.bader@canonical.com>

-apw

Patch

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 731fce6..02eb135 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -42,6 +42,8 @@ 
 
 #define FBPIXMAPSIZE	(1024 * 8)
 
+/* Protects the registered framebuffer list and count. */
+static DEFINE_SPINLOCK(registered_lock);
 struct fb_info *registered_fb[FB_MAX] __read_mostly;
 int num_registered_fb __read_mostly;
 
@@ -694,9 +696,7 @@  static ssize_t
 fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
 	unsigned long p = *ppos;
-	struct inode *inode = file->f_path.dentry->d_inode;
-	int fbidx = iminor(inode);
-	struct fb_info *info = registered_fb[fbidx];
+	struct fb_info * const info = file->private_data;
 	u32 *buffer, *dst;
 	u32 __iomem *src;
 	int c, i, cnt = 0, err = 0;
@@ -705,19 +705,28 @@  fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	if (!info || ! info->screen_base)
 		return -ENODEV;
 
-	if (info->state != FBINFO_STATE_RUNNING)
-		return -EPERM;
+	if (!lock_fb_info(info))
+		return -ENODEV;
 
-	if (info->fbops->fb_read)
-		return info->fbops->fb_read(info, buf, count, ppos);
+	if (info->state != FBINFO_STATE_RUNNING) {
+		err = -EPERM;
+		goto out_fb_info;
+	}
+
+	if (info->fbops->fb_read) {
+		err = info->fbops->fb_read(info, buf, count, ppos);
+		goto out_fb_info;
+	}
 	
 	total_size = info->screen_size;
 
 	if (total_size == 0)
 		total_size = info->fix.smem_len;
 
-	if (p >= total_size)
-		return 0;
+	if (p >= total_size) {
+		err = 0;
+		goto out_fb_info;
+	}
 
 	if (count >= total_size)
 		count = total_size;
@@ -727,8 +736,10 @@  fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 
 	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
 			 GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
+	if (!buffer) {
+		err = -ENOMEM;
+		goto out_fb_info;
+	}
 
 	src = (u32 __iomem *) (info->screen_base + p);
 
@@ -759,19 +770,21 @@  fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 		cnt += c;
 		count -= c;
 	}
+	if (!err)
+		err = cnt;
 
 	kfree(buffer);
+out_fb_info:
+	unlock_fb_info(info);
 
-	return (err) ? err : cnt;
+	return err;
 }
 
 static ssize_t
 fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 {
 	unsigned long p = *ppos;
-	struct inode *inode = file->f_path.dentry->d_inode;
-	int fbidx = iminor(inode);
-	struct fb_info *info = registered_fb[fbidx];
+	struct fb_info * const info = file->private_data;
 	u32 *buffer, *src;
 	u32 __iomem *dst;
 	int c, i, cnt = 0, err = 0;
@@ -780,8 +793,13 @@  fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 	if (!info || !info->screen_base)
 		return -ENODEV;
 
-	if (info->state != FBINFO_STATE_RUNNING)
-		return -EPERM;
+	if (!lock_fb_info(info))
+		return -ENODEV;
+
+	if (info->state != FBINFO_STATE_RUNNING) {
+		err = -EPERM;
+		goto out_fb_info;
+	}
 
 	if (info->fbops->fb_write)
 		return info->fbops->fb_write(info, buf, count, ppos);
@@ -791,8 +809,10 @@  fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 	if (total_size == 0)
 		total_size = info->fix.smem_len;
 
-	if (p > total_size)
-		return -EFBIG;
+	if (p > total_size) {
+		err = -EFBIG;
+		goto out_fb_info;
+	}
 
 	if (count > total_size) {
 		err = -EFBIG;
@@ -808,8 +828,10 @@  fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 
 	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
 			 GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
+	if (!buffer) {
+		err = -ENOMEM;
+		goto out_fb_info;
+	}
 
 	dst = (u32 __iomem *) (info->screen_base + p);
 
@@ -843,10 +865,14 @@  fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 		cnt += c;
 		count -= c;
 	}
+	if (cnt)
+		err = cnt;
 
 	kfree(buffer);
+out_fb_info:
+	unlock_fb_info(info);
 
-	return (cnt) ? cnt : err;
+	return err;
 }
 
 int
@@ -1321,8 +1347,7 @@  static long fb_compat_ioctl(struct file *file, unsigned int cmd,
 static int
 fb_mmap(struct file *file, struct vm_area_struct * vma)
 {
-	int fbidx = iminor(file->f_path.dentry->d_inode);
-	struct fb_info *info = registered_fb[fbidx];
+	struct fb_info * const info = file->private_data;
 	struct fb_ops *fb = info->fbops;
 	unsigned long off;
 	unsigned long start;
@@ -1334,6 +1359,11 @@  fb_mmap(struct file *file, struct vm_area_struct * vma)
 	if (!fb)
 		return -ENODEV;
 	mutex_lock(&info->mm_lock);
+	if (info->state == FBINFO_STATE_REMOVED) {
+		mutex_unlock(&info->mm_lock);
+		return -ENODEV;
+	}
+
 	if (fb->fb_mmap) {
 		int res;
 		res = fb->fb_mmap(info, vma);
@@ -1369,6 +1399,34 @@  fb_mmap(struct file *file, struct vm_area_struct * vma)
 	return 0;
 }
 
+static struct fb_info *get_framebuffer_info(int idx)
+__acquires(&registered_lock)
+__releases(&registered_lock)
+{
+	struct fb_info *fb_info;
+
+	spin_lock(&registered_lock);
+	fb_info = registered_fb[idx];
+	fb_info->ref_count++;
+	spin_unlock(&registered_lock);
+
+	return fb_info;
+}
+
+static void put_framebuffer_info(struct fb_info *fb_info)
+__acquires(&registered_lock)
+__releases(&registered_lock)
+{
+	int keep;
+
+	spin_lock(&registered_lock);
+	keep = --fb_info->ref_count;
+	spin_unlock(&registered_lock);
+	
+	if (!keep && fb_info->fbops->fb_destroy)
+		fb_info->fbops->fb_destroy(fb_info);
+}
+
 static int
 fb_open(struct inode *inode, struct file *file)
 __acquires(&info->lock)
@@ -1380,13 +1438,17 @@  __releases(&info->lock)
 
 	if (fbidx >= FB_MAX)
 		return -ENODEV;
-	info = registered_fb[fbidx];
+	info = get_framebuffer_info(fbidx);
 	if (!info)
 		request_module("fb%d", fbidx);
-	info = registered_fb[fbidx];
+	info = get_framebuffer_info(fbidx);
 	if (!info)
 		return -ENODEV;
 	mutex_lock(&info->lock);
+	if (info->state == FBINFO_STATE_REMOVED) {
+		res = -ENODEV;
+		goto out;
+	}
 	if (!try_module_get(info->fbops->owner)) {
 		res = -ENODEV;
 		goto out;
@@ -1403,6 +1465,8 @@  __releases(&info->lock)
 #endif
 out:
 	mutex_unlock(&info->lock);
+	if (res)
+		put_framebuffer_info(info);
 	return res;
 }
 
@@ -1418,6 +1482,7 @@  __releases(&info->lock)
 		info->fbops->fb_release(info,1);
 	module_put(info->fbops->owner);
 	mutex_unlock(&info->lock);
+	put_framebuffer_info(info);
 	return 0;
 }
 
@@ -1565,6 +1630,7 @@  register_framebuffer(struct fb_info *fb_info)
 	fb_info->node = i;
 	mutex_init(&fb_info->lock);
 	mutex_init(&fb_info->mm_lock);
+	fb_info->ref_count = 1;
 
 	fb_info->dev = device_create(fb_class, fb_info->device,
 				     MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
@@ -1608,7 +1674,6 @@  register_framebuffer(struct fb_info *fb_info)
 	return 0;
 }
 
-
 /**
  *	unregister_framebuffer - releases a frame buffer device
  *	@fb_info: frame buffer info structure
@@ -1643,6 +1708,16 @@  unregister_framebuffer(struct fb_info *fb_info)
 		return -ENODEV;
 	event.info = fb_info;
 	ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
+	if (!ret) {
+		mutex_lock(&fb_info->mm_lock);
+		/*
+		 * We must prevent any operations for this transition, we
+		 * already have info->lock so grab the info->mm_lock to hold
+		 * the remainder.
+		 */
+		fb_info->state = FBINFO_STATE_REMOVED;
+		mutex_unlock(&fb_info->mm_lock);
+	}
 	unlock_fb_info(fb_info);
 
 	if (ret) {
@@ -1662,8 +1737,7 @@  unregister_framebuffer(struct fb_info *fb_info)
 	fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
 
 	/* this may free fb info */
-	if (fb_info->fbops->fb_destroy)
-		fb_info->fbops->fb_destroy(fb_info);
+	put_framebuffer_info(fb_info);
 done:
 	return ret;
 }
diff --git a/include/linux/fb.h b/include/linux/fb.h
index e7445df..0119891 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -815,6 +815,7 @@  struct fb_tile_ops {
 struct fb_info {
 	int node;
 	int flags;
+	int ref_count;
 	struct mutex lock;		/* Lock for open/release/ioctl funcs */
 	struct mutex mm_lock;		/* Lock for fb_mmap and smem_* fields */
 	struct fb_var_screeninfo var;	/* Current var */
@@ -854,6 +855,7 @@  struct fb_info {
 	void *pseudo_palette;		/* Fake palette of 16 colors */ 
 #define FBINFO_STATE_RUNNING	0
 #define FBINFO_STATE_SUSPENDED	1
+#define FBINFO_STATE_REMOVED	2
 	u32 state;			/* Hardware state i.e suspend */
 	void *fbcon_par;                /* fbcon use-only private area */
 	/* From here on everything is device dependent */