linux-kernel - Re: Stupid VFS name lookup interface..

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 for Android: free password hash cracker in your pocket

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

Date:	Sat, 25 May 2013 17:57:10 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Eric Paris <eparis@...hat.com>,
	James Morris <james.l.morris@...cle.com>
Subject: Re: Stupid VFS name lookup interface..

On Fri, May 24, 2013 at 08:21:08PM -0700, Linus Torvalds wrote:
> On Tue, May 21, 2013 at 3:22 PM, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > Untested patch attached. It compiles cleanly, looks sane, and most of
> > it is just making the function prototypes look much nicer. I think it
> > works.
> 
> Ok, here's another patch in the "let's make the VFS go faster series".
> This one, sadly, is not a cleanup.
> 
> The concept is simple: right now the inode->i_security pointer chasing
> kills us on inode security checking with selinux. So let's move two of
> the fields from the selinux security fields directly into the inode.
> So instead of doing "inode->i_security->{sid,sclass}", we can just do
> "inode->{i_sid,i_sclass}" directly.
> 
> It's a very mechanical transform, so it should all be good, but the
> reason I don't much like it is that I think other security models
> might want to do something like this too, and right now it's
> selinux-specific. I could imagine making it just an anonymous union of
> size 64 bits or something, and just making one of the union entries be
> an (anonymous) struct with those two fields. So it's not conceptually
> selinux-specific, but right now it's pretty much a selinux hack.
> 
> But it's a selinux-specific hack that really does matter. The
> inode_has_perm() and selinux_inode_permission() functions show up
> pretty high on kernel profiles that do a lot of filename lookup, and
> it's pretty much all just that i_security pointer chasing and extra
> cache miss.
> 
> With this, inode->i_security is not very hot any more, and we could
> move the i_security pointer elsewhere in the inode.
> 
> Comments? I don't think this is *pretty* (and I do want to repeat that
> it's not even tested yet), but I think it's worth it. We've been very
> good at avoiding extra pointer dereferences in the path lookup, this
> is one of the few remaining ones.

Well...   The problem I see here is not even selinux per se - it's that
"LSM stacking" insanity.  How would your anon union deal with that?  Which
LSM gets to play with it when we have more than one of those turds around?
--
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/