linux-kernel - Re: [PATCH 0/2] ima: policy search speedup

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: <CACLa4pt698L4+BFSErV_rrmosmU-ELYZcmSBR8JqtYBTN35z9g@mail.gmail.com>
Date:	Tue, 11 Dec 2012 13:09:25 -0500
From:	Eric Paris <eparis@...isplace.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	"Kasatkin, Dmitry" <dmitry.kasatkin@...el.com>,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	LSM List <linux-security-module@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	James Morris <jmorris@...ei.org>
Subject: Re: [PATCH 0/2] ima: policy search speedup

On Tue, Dec 11, 2012 at 12:55 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:

> And your "pseudo-filesystems" argument is pretty stupid too, since WE
> ALREADY HAVE A FLAG FOR THAT!
>
> Guess where it is? Oh, it's in the place I already mentioned makes
> more sense. Look for S_PRIVATE in inode->i_flags, and IS_PRIVATE() in
> users. It's what the other security models already use to avoid
> bothering calling down to the security layers. The fact that the
> integrity layer bypasses the normal security layer in
> ima_file_check(), for example, is no excuse to then make up totally
> new flags.

IS_PRIVATE() is not used by and darn well better not be used by, all
psuedo filesystems like procfs which IMA may want to ignore.  LSMs
like to do control on them.  I thought S_PRIVATE was really only used
by the anon_inode and reiserfs's really crazy ass internal inodes.  I
could always be wrong.

I don't know if I agree with dmitry but let me explain what's going on here.

Lets say the user accesses an inode in procfs.  Without this patch one
would search the ima policy and find a rule that says 'if this inode
is on procfs we don't care.'  We can then cache that in the struct
inode like you say and move along.  If another inode on procfs is
opened we will have the same policy search and the same per inode 'i
don't give a crap' marking.  This absolutely works you are right.  But
we search the IMA policy for every inode.

With Dmitry's patch we can instead search the IMA policy one time and
mark the whole superblock as 'i don't give a crap' if IMA policy says
we don't care about that fstype.  When the second procfs inode is
opened we instead look at the per superblock 'who gives a crap' and
never search the IMA policy.  So we save all future policy searches.

I'd say this patch would only be a good idea if there is a real
performance hit which is measurable in a real work situation.  Not,
'look how much faster it is to access proc inodes' microbenchmark,
since noone is actually going to do that, but some results of a useful
benchmark you care about.  Maybe Dmitry gave those numbers and I
missed them?  Otherwise, stick with per inode like Linus wants...
--
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/