linux-kernel - Re: [RFC] situation with fput() locking (was 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:	Fri, 27 Apr 2012 10:35:25 +0300
From:	"Kasatkin, Dmitry" <dmitry.kasatkin@...el.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Al Viro <viro@...iv.linux.org.uk>, Hugh Dickins <hughd@...gle.com>,
	linux-fsdevel@...r.kernel.org, James Morris <jmorris@...ei.org>,
	linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	David Safford <safford@...ux.vnet.ibm.com>,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	David Miller <davem@...emloft.net>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [RFC] situation with fput() locking (was Re: [PULL REQUEST] :
 ima-appraisal patches)

On Sat, Apr 21, 2012 at 1:35 AM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Fri, Apr 20, 2012 at 3:13 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
> >
> > Kinda-sorta.  I agree that such helpers make sense, but you are too
> > optimistic about the number of such places.  And clusterfuck around
> > mremap() is fairly deep, so propagating all way back to up_write()
> > wont' be fun.
>
> Fair enough.
>
> I'll do the helpers and see how much they get rid of, just because
> looking at all the callers, those helpers seem to be obviously the
> right thing anyway. So even if we don't do anything else, we can
> improve things regardless.
>
> For do_brk(), for example, it looks like do_brk() itself should
> actually be entirely static to mm/mmap.c, because every single caller
> from the outside actually wants the self-locking version.
>
> So plan right now: do "vm_xyzzy()" helper functions that do
> "do_xyzzy()" and take the lock (and do not take the "mm" argument,
> because it had better always be the current one - keep the calling
> convention as simple as possible).
>
>                   Linus

Moi Linus,

Taking the list of files to fput out of do_munmap() and doing it after
unlocking mmap sem
was one of possible solutions when we discussed it with Mimi.
But we were looking for the solution with less modifications of existing kernel.

In fact IMA is already doing some work before taking mmap sem.
---
	ret = ima_file_mmap(filep, prot);
	if (ret < 0)
		return ret;
	down_write(&current->mm->mmap_sem);
--

But have you seen the proposed patch for __fput()?
[PATCH v4 10/12] ima: defer calling __fput()

It defers only of course the last AND mmap_sem is locked AND open for write.

	if (current->mm && rwsem_is_locked(&current->mm->mmap_sem)) {
		if (ima_defer_fput(file) == 0)
			return;
	}

Just 5 out of ~100,000 mmap_sem held fput() calls were deferred.

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