Re: [PATCH 1/3] IMA: move read/write counters into struct inode

[Ingo Molnar <mingo@elte.hu>]

* Eric Paris <eparis@redhat.com> wrote:

> More cryptic command line options and complexity is not the solution.  I have no 
> plans to send (a fixed/"working" version of) Kyle's patch which would cause a 
> userspace regression since working machines would magically stop working.

That is a valid argument.

> The right solution is to provide features without unreasonably impacting those who 
> do not want those features.  At the moment my patch series reduces memory usage by 
> a factor of at least 40 and eliminates the locking contention and serialization of 
> bringing inodes into and out of core.  It does so without introducing ANY 
> additional overhead or complexity.

And that argument is missing the point IMO. Christoph is perfectly correct to 
observe:

  It bloats the inode with three unsigned long values for a feature no sane person
  would ever use.

that's 24 bytes per inode - when in the past we've made _huge_ efforts to minimally 
shrink it by a few bytes.

> If there is a general consensus that 24 bytes per inode is too large we can move 
> forward from here and drop the 'opencount' and save 8 bytes (while eliminating the 
> debugging and verification this code has helped to provide in the past)
> 
> If there is a general consensus that 16 bytes per inode is too large we can travel 
> down the much more complex route of attempting to collect this information 
> dynamically by freezing userspace, scanning every open file, and calculating the 
> information.  We would then have a 2 stage integrity structure, the first with 
> just counters and the second would contain integrity information if needed.

Hm, why would all that be needed?

The security subsystem already has a private-data extension in struct inode:

 struct inode {
 ...
 #ifdef CONFIG_SECURITY
         void                    *i_security;
 #endif
 ...
 }

That could be used for IMA, right? IMA state can either embedd in struct 
inode_security_struct or the security subsystem could define an intermediate data 
structure. (it's up to the security subsystem how it wants to make use of this data 
pointer.)

An unlimited amount of optional information can be attached that way.

Thanks,

	Ingo