linux-kernel - Re: [PATCH 3/5] tty: handle VT specific compat ioctls in vt driver

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]

Date:	Fri, 7 Aug 2009 09:04:11 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	linux-kernel@...r.kernel.org, Christoph Hellwig <hch@....de>,
	Andi Kleen <andi@...stfloor.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	"Greg Kroah-Hartman" <gregkh@...e.de>
Subject: Re: [PATCH 3/5] tty: handle VT specific compat ioctls in vt driver

On Friday 07 August 2009, Frederic Weisbecker wrote:

> On Thu, Aug 06, 2009 at 03:09:28PM +0200, Arnd Bergmann wrote:
> > The VT specific compat_ioctl handlers are the only ones
> > in common code that require the BKL. Moving them into
> > the vt driver lets us remove the BKL from the other handlers
> > and cleans up the code.
> 
> 
> Why does it require the bkl?
> 

All the VT ioctls are currently called under the BKL. I did not try
to find out why that is but simply kept that state. All other
compat ioctl do not interact with device driver state at all,
so they obviously do not need the BKL.

> > +
> > +long vt_compat_ioctl(struct tty_struct *tty, struct file * file,
> > +	     unsigned int cmd, unsigned long arg)
> > +{
> > +	struct vc_data *vc = tty->driver_data;
> > +	struct console_font_op op;	/* used in multiple places here */
> > +	struct kbd_struct *kbd;
> > +	unsigned int console;
> > +	void __user *up = (void __user *)arg;
> > +	int perm;
> > +	int ret = 0;
> > +
> > +	console = vc->vc_num;
> > +
> > +	lock_kernel();
> 
> 
> 
> It would be really nice to add a comment here that explain what it
> is protecting.
> I would like to work on removing the bkl from tty, and such nude lock_kernel()
> don't help much to work in this area.
> This is more than ever a FUD lock, nothing about its role in tty in the Lockronomicon,
> and even not in the comments :-)

This function is a straight copy from vt_ioctl, with the data structures replaced,
and calling it vt_ioctl where they are identical.

You are right that this needs more code comments to make that obvious.

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