Re: [PULL REQUEST] : ima-appraisal patches

From: Al Viro
Date: Wed Apr 18 2012 - 14:39:40 EST

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@xxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at