linux-kernel - Re: [PATCH 6/6] procfs: Kill the bkl in ioctl

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: <20100411130341.GA6353@infradead.org>
Date:	Sun, 11 Apr 2010 09:03:41 -0400
From:	Christoph Hellwig <hch@...radead.org>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	Arnd Bergmann <arnd@...db.de>,
	Stefan Richter <stefanr@...6.in-berlin.de>,
	Alexey Dobriyan <adobriyan@...il.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	John Kacur <jkacur@...hat.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Al Viro <viro@...iv.linux.org.uk>, Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH 6/6] procfs: Kill the bkl in ioctl

On Sat, Apr 10, 2010 at 05:28:16PM +0200, Frederic Weisbecker wrote:
> So you mean we should attribute explicit default_llseek to the evil
> places instead of explicit generic_file_llseek in the safe ones?
> That's not a bad idea as it would result in much less changes.
> 
> The problem happens the day you switch to generic_file_llseek() as the
> new default llseek(), how do you prove that all remaining fops
> that don't implement .llseek don't use the bkl? There will be
> hundreds of them and saying "we've looked all of them and they don't
> need it" will be a scary justification.
> 
> On the opposite, attributing explicit generic_file_llseek or
> non_seekable_open on the safe places and default_llseek on
> the dozens of others doubtful places is easier to get a
> safe conclusion.
> 
> But yeah we should try, at least attributing explicit
> default_llseek won't harm, quite the opposite.

Note that an lssek that actually does something is the wrong default,
even if we have it that way currently.  If the default is changed it
should be changed to give the semantics that nonseekable_open()
gives us.  Given that you guys are so motivated to do something in
this area it might be a good idea to do this in a few simple steps:

 - make sure every file operation either has a ->llseek instead or
   calls nonseekable_open from ->open
 - remove nonseekable_open and all calls to it
 - switch all users of no_llseek to not set a ->llsek after auditing
   that there's no corner case where we want to allow pread/pwrite
   but not lseek, which is rather unlikely
 - walk through the instances now using default_llseek and chose
   a better implementation for this particular instance.  Often
   this will be just removing the the lssek method as not allowing
   seeks is the right thing to do for character drivers, even if it
   is a behaviour change from the current version which usually
   is the result of sloppy coding.

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