linux-kernel - Re: [CFT] Re: VFS deadlock ?

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, 22 Mar 2013 12:43:50 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Al Viro <viro@...iv.linux.org.uk>
Cc:	Dave Jones <davej@...hat.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: Re: [CFT] Re: VFS deadlock ?

On Thu, Mar 21, 2013 at 10:33 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> And I wonder how big of a deal the aggressive dentry deletion is.
> Maybe it's even ok to allocate/free the inodes all the time. The whole
> "get the inode hash lock and look it up there" can't be all that
> wonderful either. It takes the inode->i_lock for each entry it finds
> on the hash list, which looks horrible. I suspect our slab allocator
> isn't much worse than that, although the RCU freeing of the inodes
> could end up being problematic.

I tested this out by just having a process that keeps stat'ing the
same /proc inode over and over and over again, which should be pretty
much the worst-case situation (because we will delete the dentry after
each stat, and so we'll repopulate it for each stat)

The allocation overhead seems to be in the noise. The real costs are
all in proc_lookup_de() just finding the entry, with strlen/strcmp
being much hotter. So if we actually care about performance for these
cases, what we would actually want to do is to just cache the dentries
and have some kind of timeout instead of getting rid of them
immediately. That would get rid of the lookup costs, and in the
process also get rid of the constant inode allocation/deallocation
costs.

There was also some locking overhead, but that's also mainly
dentry-related, with the inode side being visible but not dominant.
Again, not immediately expiring the dentries would make all that just
go away.

Regardless, the patch seems to be the right thing to do. It actually
simplifies /proc, seems to fix the problem for Dave, and is small and
should be trivial to backport. I've committed it and marked it for
stable. Let's hope there won't be any nasty surprises, but it does
seem to be the simplest non-hacky patch.

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