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] [day] [month] [year] [list]

Message-ID: <20110609063810.GA8071@localhost>
Date:	Thu, 9 Jun 2011 14:38:10 +0800
From:	Wu Fengguang <fengguang.wu@...el.com>
To:	Bruno Prémont <bonbons@...ux-vserver.org>
Cc:	"Antonino A. Daplas" <adaplas@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>, thomas.creutz@....de
Subject: Re: [RFC] fbmem: reset file->private_data on failed fb_open()

[Add CC to Thomas Creutz]

On Thu, Jun 09, 2011 at 02:10:19PM +0800, Bruno Prémont wrote:
> 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.

That's great! Then hopefully the bug can be closed?

> > 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...

Good, thanks for the confirmation.

Thanks,
Fengguang

> > ---
> >  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/