linux-kernel - Re: [PULL REQUEST] : ima-appraisal patches

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, 18 Apr 2012 19:39:38 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Mimi Zohar <zohar@...ux.vnet.ibm.com>
Cc:	James Morris <jmorris@...ei.org>,
	linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	David Safford <safford@...ux.vnet.ibm.com>,
	Dmitry Kasatkin <dmitry.kasatkin@...el.com>
Subject: Re: [PULL REQUEST] : ima-appraisal patches

On Wed, Apr 18, 2012 at 02:07:52PM -0400, Mimi Zohar wrote:

> >From the 'ima: defer calling __fput()' patch description:
> 
> ima_file_free(), which is called on __fput(), updates the file data
> hash stored as an extended attribute to reflect file changes.  If a
> file is closed before it is munmapped, __fput() is called with the
> mmap_sem taken.  With IMA-appraisal enabled, this results in an
> mmap_sem/i_mutex lockdep.  ima_defer_fput() increments the f_count to
> defer the __fput() being called until after the mmap_sem is released.
> 
> The number of __fput() calls needing to be deferred is minimal.  Only
> those files in policy, that were closed prior to the munmap and were
> mmapped write, need to defer the __fput().
> 
> With this patch, on a clean F16 install, from boot to login, only
> 5 out of ~100,000 mmap_sem held fput() calls were deferred.

Assuming that it's commit 3cee52ffe8ca925bb1e96f804daa87f7e2e34e46
Author: Mimi Zohar <zohar@...ux.vnet.ibm.com>
Date:   Fri Feb 24 06:23:12 2012 -0500

    ima: defer calling __fput()
in your tree, the NAK still stands.  For starters, but you are creating a
different locking rules for IMA-enabled builds and for everything else.
Moreover, this deferral is done only for files opened for write; the
rules are convoluted as hell *and* inviting abuses.  

NAKed at least until you come up with formal proof that there's no other
lock where fput() would be possible and ->i_mutex was not allowed.  This
is not a way to go; that kind of kludges leads to locking code that is
impossible to reason about.

PS: BTW, what the hell is "fput already scheduled" codepath about?
Why is it pr_info() and not an outright BUG_ON()?
--
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/