linux-kernel - Re: ima: use of radix tree cache indexing == massive waste of memory?

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:	Mon, 18 Oct 2010 12:48:54 -0400
From:	Eric Paris <eparis@...hat.com>
To:	Kyle McMartin <kyle@...artin.ca>
Cc:	James Morris <jmorris@...ei.org>,
	Christoph Hellwig <hch@...radead.org>,
	kernel@...ts.fedoraproject.org, Mimi Zohar <zohar@...ibm.com>,
	warthog9@...nel.org, Dave Chinner <david@...morbit.com>,
	linux-kernel@...r.kernel.org, "H. Peter Anvin" <hpa@...or.com>,
	Serge Hallyn <serue@...ibm.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>, mingo@...e.hu
Subject: Re: ima: use of radix tree cache indexing == massive waste of
 memory?

On Mon, 2010-10-18 at 02:25 -0400, Kyle McMartin wrote:
> If someone gives me a good reason why Fedora actually needs this
> enabled, I'm going to apply the following patch to our kernel so that
> IMA is globally an opt-in feature... Otherwise I'm inclined to just
> disable it.

I'll can address this on the fedora list, but I think this is the wrong
approach.  IMA is supposed to be of negligible impact when not 'enabled'
and I believe the right solution is to fix places where that isn't true.
At the moment 3 have been identified.

1) IMA uses radix trees which end up wasting 500 bytes per inode because
the key is too sparse.  I've got a patch which uses an rbtree instead
I'm testing and will send along shortly.  I found it funny working on
the patch to see that Documentation/rbtree.txt says "This differs from
radix trees (which are used to efficiently store sparse arrays and thus
use long integer indexes to insert/access/delete nodes)"  Which flys in
the face of this report.

2) IMA creates an entire integrity structure for every inode even when
most or all of this structure will not be needed.

3) IMA serializes bringing inodes into and out of core because it takes
a spinlock() when adding or removing the integrity structure to the
radix/rbtree.

I've got a patch for #1 and think 2 and 3 are relatively simple to fix
as well.  Certainly #2 is, and I think that will lead to a solution to
#3.

> diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
> index afba4ae..767f026 100644
> --- a/security/integrity/ima/ima_iint.c
> +++ b/security/integrity/ima/ima_iint.c

> @@ -139,6 +139,11 @@ static void init_once(void *foo)
>  
>  static int __init ima_iintcache_init(void)
>  {
> +	extern int ima_enabled;

extern in a .c file?  didn't you already expose this in the
appropriate .h file?

> +
> +	if (!ima_enabled)
> +		return 0;
> +
>  	iint_cache =
>  	    kmem_cache_create("iint_cache", sizeof(struct ima_iint_cache), 0,
>  			      SLAB_PANIC, init_once);
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index e662b89..92e084c 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -26,6 +26,7 @@
>  #include "ima.h"
>  
>  int ima_initialized;
> +int ima_enabled = 0;

initializing a global?

>  
>  char *ima_hash = "sha1";
>  static int __init hash_setup(char *str)

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