Re: Stupid VFS name lookup interface..

[Casey Schaufler]

On 5/25/2013 9:57 AM, Al Viro wrote:
> 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@xxxxxxxxxxxxxxxxxxxx> 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?

I don't know that the terms "insanity" and "turds" really do
the situation justice, but Al has a firm grasp on the nut of the issue.

The LSM stacking I've been working on (v14 due "real soon") would
render this change useless, as you'd have to have the multiple
instances of the special fields just as you'd need multiple blob
pointers. That would have to reintroduce the indirection you're
trying to be rid of. I have been working on the assumption that
the single blob pointer was all that could ever go into the inode.
If that's not true stacking could get considerably easier and could
have less performance impact.

Now I'll put on my Smack maintainer hat. Performance improvement is
always welcome, but I would rather see attention to performance of
the LSM architecture than SELinux specific hacks. The LSM blob
pointer scheme is there so that you (Linus) don't have to see the
dreadful things that we security people are doing. Is it time to
get past that level of disassociation? Or, and I really hate asking
this, have you fallen into the SELinux camp?


> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/