linux-kernel - Re: 2.6.27-rc7-sha1: EIP at proc_sys

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 28 Sep 2008 13:38:27 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
cc:	Alexey Dobriyan <adobriyan@...il.com>, viro@...iv.linux.org.uk,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: 2.6.27-rc7-sha1: EIP at proc_sys_compare+0x36/0x50

On Sat, 27 Sep 2008, Eric W. Biederman wrote:
> 
> We hold the directory mutex in real_lookup
> before calling i_op->lookup.

Irrelevant.

The d_compare function si called _before_ we get the directory mutex. It's 
done purely under dentry->dlock (and the RCU read lock).

So we have absolutely no directory-level locking here.

> So lookups should be serialized.

lookups are serialized before calling into the filesystem with ->lookup, 
but _not_ at the d_compare level. If we serialized d_compare, the dentry 
cache would be no use at all, we'd serialize all lookups, cached or not.

(Of course, sane filesystems will not have d_compare at all, just the 
memcmp, but we're talking /proc here - although I forget why it wants to 
do that insane d_compare thing)

So forget the directory mutex. d_compare is much more low-level than that. 
It can hit a dentry that is being unhashed at the same time for whatever 
reason (memory pressure, whatever).

The fact is, the "->lookup()" function is meant to be easy for filesystems 
to use, but d_compare i really low-level dentry magic and is meant to look 
at just the *name*. It's meant to be a replacement for memcpy() for things 
like case-independent comparisons or strange utf-8 rules (ie "equivalent" 
characters). It has no real locking, since the names are supposed to be 
"stable" anyway (the dentry->d_lock should guarantee that the dentry isn't 
being renamed etc).

I may be missing something, of course, but the dentry eas actually found 
_before_ takign even the dentry->d_lock, so the dentry we call compare on 
may not even be *valid* any more, because something might have unhashed it 
_after_ we found it, but _before_ we got d_lock.

d_compare() really is pretty special (d_revalidate is too, for that 
matter, although at least _slightly_ less so: by that time we have at 
least tested that the UNHASHED bit isn't set because we raced with 
removal etc).

		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/