Re: inode_permission NULL pointer dereference in 3.13-rc1

[Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>]

On Thu, Nov 28, 2013 at 3:44 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
>         * d_count(dentry) is -128
>         * dentry->d_inode is NULL
>
> In other words, what we get is an extra dput() somewhere.  The trouble is,
> all likely places I'm seeing in the "RCU'd vfsmounts" seem to be OK...
> In theory, we might be hitting a _missing_ dput(), with counter wrapping
> around, but that doesn't seem likely...

So d_count = -128 means that it's dead (see lockref_mark_dead). So it
goes from 0 (last refcount entry) to dead when it transitions into
dentry_kill. Which explains the inode being NULL too, because that
means it's gone through dentry_iput() as well.

And if it was just a normal dentry being passed around as the result
of a lookup, then because we still have LOOKUP_RCU set, such a dentry
is technically "valid" - it just hasn't gotten to the point where
we'll fail it.

HOWEVER. It's certainly *not* valid if "current->fs->root/pwd" points
to it. So yeah, there must have been an extra dput() somewhere. Or,
more likely, I think, we don't get the refcount to some dentry
properly any more.

I don't see where, though. You did change where "LOOKUP_RCU" is
cleared in unlazy_walk() but you did add that

        nd->path.dentry = NULL;

and that looks like it should be ok. And I don't see what else would care.

                   Linus