linux-kernel - Re: [RFC] fbmem: reset file->private_data on failed fb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110609081019.145ed6d5@pluto.restena.lu>
Date:	Thu, 9 Jun 2011 08:10:19 +0200
From:	Bruno Prémont <bonbons@...ux-vserver.org>
To:	Wu Fengguang <fengguang.wu@...el.com>
Cc:	"Antonino A. Daplas" <adaplas@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-fbdev@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] fbmem: reset file->private_data on failed fb_open()

On Thu, 9 Jun 2011 11:06:28 Wu Fengguang <fengguang.wu@...el.com> wrote:
> I wrote this when looking at NULL dereference bug
> https://bugzilla.kernel.org/show_bug.cgi?id=18912

The trace over there rather looks like closing of a framebuffer that
has been replaced (and destroyed) during boot sequence.
This action happens when switching from VESA to KMS from initrd
or early userspace boot sequence where plymouthd had already opened
vesafb.

That should hopefully be fixed by refcounting FBs as done during
2.6.39-rc7.

> Will it help by clearing private_data? I have no idea at all, because
> for regular files, ->release won't be called on failed ->open. Just in
> case there are some exceptions in fbmem...

fb devices files should be just as regular as any other device files...

> ---
>  drivers/video/fbmem.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> --- linux-next.orig/drivers/video/fbmem.c	2011-06-09 10:36:06.000000000 +0800
> +++ linux-next/drivers/video/fbmem.c	2011-06-09 10:39:30.000000000 +0800
> @@ -1424,26 +1424,28 @@ __releases(&info->lock)
>  	file->private_data = info;
>  	if (info->fbops->fb_open) {
>  		res = info->fbops->fb_open(info,1);
>  		if (res)
>  			module_put(info->fbops->owner);
>  	}
>  #ifdef CONFIG_FB_DEFERRED_IO
>  	if (info->fbdefio)
>  		fb_deferred_io_open(info, inode, file);
>  #endif
>  out:
>  	mutex_unlock(&info->lock);
> -	if (res)
> +	if (res) {
> +		file->private_data = NULL;
>  		put_fb_info(info);
> +	}
>  	return res;
>  }
>  
>  static int 
>  fb_release(struct inode *inode, struct file *file)
>  __acquires(&info->lock)
>  __releases(&info->lock)
>  {
>  	struct fb_info * const info = file->private_data;
>  
>  	mutex_lock(&info->lock);
>  	if (info->fbops->fb_release)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/