Messages in this thread | | | Date | Sun, 28 Sep 2008 13:38:27 -0700 (PDT) | From | Linus Torvalds <> | 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
| |