Re: [GIT][IMA] fix null pointer deref

[Al Viro <viro@ZenIV.linux.org.uk>]

On Sun, Feb 07, 2010 at 10:42:29PM -0500, Mimi Zohar wrote:

> > ima_path_check() side is also of BUG_ON() variety (and isn't triggered).
> 
> hm, there was quite a bit of discussion that IMA should be called from
> the security hooks (refer to http://lkml.org/lkml/2009/6/7/279),  so
> commit 6c21a7f "LSM: imbed ima calls in the security hooks" moved them.

You know, I'm -><- that close to posting a highly unprintable rant about
hooks in general, associated style of development and resulting problems.
With names named and *many* examples given.

LSM is essentially a trashcan and just about everything icky gets swept
over there.  That's fine, as long as one doesn't care whether their code
makes sense and just wants to keep it away from unfriendly eyes.

The first question one should ask is "what would be the natural point in
object life cycle to do that", not "which hooks can I plug that into and
how do I work around this, this and that".

In this case, "when is struct file freed?" became a proxy for "when is
an opened file closed?", with bogus heuristics added to distinguish the
callers.  Nothing in VFS has promised NULL ->f_path.dentry for a struct
file that hasn't been fully set up (i.e. hasn't transitioned to a state
where fput() is the proper destructor).  The fact that you guys ran into
a situation where you needed that kind of heuristics should've been
a clear indicator that something was wrong; silently adding them into
place that by design is outside of normal code was Wrong with capital WTF.

I'd better stop before that devolves into saying what I really think about
LSM, assorted Creatures From Black Lagoon slipping into fs/*.c (and mm/*.c)
once a year or so, et revolting cetera.  If I ever find a way to do that
adequately without incoherent obscenities, I'll post it.

The bottom line: minimizing patch footprint is a good thing, but NOT if
it comes at the cost of bogus kludges.