linux-kernel - Re: [PATCH 4/6] IMA: only allocate iint when needed

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:	Wed, 20 Oct 2010 04:53:24 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Eric Paris <eparis@...hat.com>
Cc:	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, hch@...radead.org, zohar@...ibm.com,
	warthog9@...nel.org, david@...morbit.com, jmorris@...ei.org,
	kyle@...artin.ca, hpa@...or.com, akpm@...ux-foundation.org,
	torvalds@...ux-foundation.org, mingo@...e.hu
Subject: Re: [PATCH 4/6] IMA: only allocate iint when needed

On Tue, Oct 19, 2010 at 06:58:33PM -0400, Eric Paris wrote:
> IMA always allocates an integrity structure to hold information about every
> inode, but only needed this structure to tract the number of readers and
> writers currently accessing a given inode.  Since that information was moved
> into struct inode instead of the integrity struct this patch stops allocating
> the integrity stucture until it is needed.  Thurs greatly reducing memory
> usage.

OK, I'm really confused.  Could you explain what's going on with refcounts
now?  AFAICS, you allocate them in process_measurement() now and they live
until the inode is torn down.  Fine by me.  However
	* why bother with bumping refcount in ima_file_free()?  It's not going
to die until we free the inode, for fsck sake...
	* i_mutex is a damn strange choice for protecting your write counter
	* for that matter, why bother with refcount at all?  What could
hold a reference to that sucker without holding a reference to struct inode?
I don't see anything of that sort, other that delayed freeing that'll hold
the only remaining pointer to iint until the callback where it'll promptly
drop it.

And while we are at it, you are forcing a hash lookup on _every_ _damn_
close(2) of a regular file.  How about marking the inode as ima-infested
so that it wouldn't bother?  For that matter, this "final writer" logics
needs explanation as well...
--
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/