Messages in this thread | | | Date | Sun, 28 Sep 2008 15:18:56 +0100 | From | Al Viro <> | Subject | Re: 2.6.27-rc7-sha1: EIP at proc_sys_compare+0x36/0x50 |
| |
On Fri, Sep 26, 2008 at 08:47:51AM -0700, Linus Torvalds wrote:
> and as far as I can tell, there is nothing to say that a /proc inode > cannot be a negative dentry. Sure, we try to get rid of them, but during a > parallel lookup, we will have added the dentry with a NULL inode in the > other lookup. > > So assuming that you have an inode at that point seems to be utter crap. > > Now, the whole _function_ is utter crap and should probably be dropped, > but whatever. That's just another sysctl insanity. In the meantime, > something like this does look appropriate, no? > > Al, did I miss something?
The real underlying bug, whatever it is. If this sucker ever becomes negative, we have a big problem. Where _could_ that happen? Remember, we do not allow ->rmdir() and ->unlink() to succeed there. So d_delete() callers in namei.c are out of question. We also never do d_add() with NULL inode in there. We _might_ be doing a bogus d_rehash() on a negative /prooc/sys/<something> dentry that had never been hashed to start with somewhere in generic code, but... I don't see where that could happen. vfs_rename_dir() with negative new_dentry would have to get it from something and that would have to be ->lookup(). And that sucker returns ERR_PTR() or a positive dentry in all cases here. d_splice_alias() is not used there at all; d_move_locked() would scream bloody murder if dentry it's rehashing is negative. d_materialize_unique() and d_add_unique() are not used. So just WTF is creating this sucker?
IOW, your patch will probably be enough to stop the visible problem, but I would dearly like to understand what's really causing it. It appears to be a refcounting breakage somewhere and we have *another* bug report that smells like that - it seems like we sometimes end up with negative dentry on alias list of an inode (outside of /proc/sys, AFAICT). Something really fishy is going on...
| |