linux-kernel - Re: [GIT][IMA] fix null pointer deref

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] [day] [month] [year] [list]

Date:	Mon, 8 Feb 2010 05:33:41 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Mimi Zohar <zohar@...ibm.com>
Cc:	Xiaotian Feng <dfeng@...hat.com>, Eric Paris <eparis@...hat.com>,
	Eugene Teo <eugene@...hat.com>,
	James Morris <jmorris@...ei.org>, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	linux-security-module-owner@...r.kernel.org,
	serue@...ux.vnet.ibm.com,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>
Subject: Re: [GIT][IMA] fix null pointer deref

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