lkml.org 
[lkml]   [2012]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@linux.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()?


\
 
 \ /
  Last update: 2012-04-18 22:37    [W:0.156 / U:0.916 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site