From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
ebiederm@xmission.com, akpm@linux-foundation.org,
linux-kernel@vger.kernel.org
Subject: Re: 2.6.27-rc7-sha1: EIP at proc_sys_compare+0x36/0x50
Date: Sun, 28 Sep 2008 15:18:56 +0100 [thread overview]
Message-ID: <20080928141856.GG28946@ZenIV.linux.org.uk> (raw)
In-Reply-To: <alpine.LFD.1.10.0809260838120.3265@nehalem.linux-foundation.org>
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...
next prev parent reply other threads:[~2008-09-28 14:19 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-26 15:20 2.6.27-rc7-sha1: EIP at proc_sys_compare+0x36/0x50 Alexey Dobriyan
2008-09-26 15:47 ` Linus Torvalds
2008-09-27 8:44 ` Eric W. Biederman
2008-09-28 20:38 ` Linus Torvalds
2008-09-28 14:18 ` Al Viro [this message]
2008-09-28 19:28 ` Hugh Dickins
2008-09-28 20:55 ` Linus Torvalds
2008-09-28 20:59 ` Linus Torvalds
2008-09-28 22:07 ` Hugh Dickins
2008-09-29 3:05 ` Eric W. Biederman
2008-09-28 20:46 ` Linus Torvalds
2008-09-28 20:50 ` Linus Torvalds
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080928141856.GG28946@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox