All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: linux-m68k@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Subject: aranym bug, manifests as "ida_remove called for id=13" on recent kernels
Date: Thu, 7 Oct 2010 18:49:48 +0100	[thread overview]
Message-ID: <20101007174948.GT19804@ZenIV.linux.org.uk> (raw)

	I've spent quite a while hunting that crap down; reverting VFS fix
mentioned in original thread *does* get rid of the symptoms, but so does the
patch below.

	What happens is this: if ->follow_link() (usually something like
stat("/proc/2/fd", ...) done by pidof(8)) return ERR_PTR(-....), we return
to __do_follow_link() and do the following:
        *p = dentry->d_inode->i_op->follow_link(dentry, nd);
        error = PTR_ERR(*p);
        if (!IS_ERR(*p)) {
                char *s = nd_get_link(nd);
                error = 0;
                if (s)
                        error = __vfs_follow_link(nd, s);
                else if (nd->last_type == LAST_BIND) {
                        error = force_reval_path(&nd->path, nd);
                        if (error)
                                path_put(&nd->path);
                }
        }
        return error;

We _should_ return non-zero value; IS_ERR(ERR_PTR(-n)) is 1 and
PTR_ERR(ERR_PTR(n)) is -n.  What happens instead is that this thing
actually returns 0.  And no, it's not a miscompile.  Patch below
removes the symptoms of the bug, but only if both parts are present.
I.e. *not* doing "report = 1" in proc_pid_follow_link() gives us
visible breakage, despite the fact that report is initialized as
1 and nothing except proc_pid_follow_link() ever tries to assign
anything to it.  Seeing that fs/namei.c and fs/proc/base.c are
compiled separately, we can exclude gcc problems.

The cheapest way to reproduce is to boot with init=/bin/sh, then
mount /proc and have stat("/proc/2/exe", &st) called; if stat()
returns 0, we are fscked.  The critical part is between return
from proc_exe_link() (we'll leave it via if (!mm) return -ENOENT;)
to return from __do_follow_link() -> do_follow_link() -> link_path_walk().

If somebody familiar with aranym guts are up to debugging that, more
power to them.  If I would've seen it on real hardware, I'd suspect
something weird going on with caches, but...

FWIW, it's observable on amd64 host; I haven't tried it on x86.  Version
of aranym is 0.9.6beta2-1 (one in lenny).  Have fun...

Patch [*NOT* for inclusion into mainline, obviously] follows:

diff --git a/fs/namei.c b/fs/namei.c
index 24896e8..da5bb7f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -524,6 +524,8 @@ static inline void path_to_nameidata(struct path *path, struct nameidata *nd)
 	nd->path.dentry = path->dentry;
 }
 
+int report = 1;
+
 static __always_inline int
 __do_follow_link(struct path *path, struct nameidata *nd, void **p)
 {
@@ -552,6 +554,8 @@ __do_follow_link(struct path *path, struct nameidata *nd, void **p)
 				path_put(&nd->path);
 		}
 	}
+	if (report && !error && IS_ERR(*p))
+		printk("fucked: %d %p\n", error, *p);
 	return error;
 }
 
diff --git a/fs/proc/base.c b/fs/proc/base.c
index a1c43e7..24579de 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1513,6 +1513,7 @@ static void *proc_pid_follow_link(struct dentry *dentry, struct nameidata *nd)
 		goto out;
 
 	error = PROC_I(inode)->op.proc_get_link(inode, &nd->path);
+	{extern int report; report = 1;}
 out:
 	return ERR_PTR(error);
 }

             reply	other threads:[~2010-10-07 17:49 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-07 17:49 Al Viro [this message]
2010-10-10  9:47 ` aranym bug, manifests as "ida_remove called for id=13" on recent kernels Geert Uytterhoeven
2010-10-10 14:49   ` Al Viro
2010-10-10 20:18     ` Geert Uytterhoeven
2010-10-10 23:52       ` Al Viro
2010-10-11  2:41         ` Al Viro
2010-10-11  4:36           ` Brad Boyer
2010-10-11  4:48             ` Al Viro
2010-10-11 12:21               ` Thorsten Glaser
2010-10-11 12:21                 ` Thorsten Glaser
2010-10-11 13:10                 ` Andreas Schwab
2010-10-11 13:35                 ` Al Viro
2010-10-11 13:45                   ` Thorsten Glaser
2010-10-11 14:15                   ` Andreas Schwab
2010-10-11 14:24                     ` Al Viro
2010-10-11 22:02                       ` Al Viro
2010-11-02 15:30                       ` Geert Uytterhoeven
2010-10-11 19:05                 ` Mikael Pettersson
2010-10-11  9:27           ` Mikael Pettersson
2010-10-11 11:50             ` Mikael Pettersson
2010-10-11 12:29             ` Andreas Schwab
2010-10-11  8:39         ` Geert Uytterhoeven

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=20101007174948.GT19804@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.