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

From: Al Viro
Date: Tue Oct 19 2010 - 23:53:30 EST


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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/