xfs
[Top] [All Lists]

Re: inode_permission NULL pointer dereference in 3.13-rc1

To: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Subject: Re: inode_permission NULL pointer dereference in 3.13-rc1
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Thu, 28 Nov 2013 18:07:27 -0800
Cc: Dave Chinner <david@xxxxxxxxxxxxx>, Christoph Hellwig <hch@xxxxxxxxxxxxx>, linux-fsdevel <linux-fsdevel@xxxxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=/rspa1hOyfj9paKWlmQeAR1NhOjB1Gg8oQ3LOCn9S6k=; b=q3xboQDZ9n3FkF7I5hCM1Hhttu1zVDEw+gH0ynRb6PkSSdmkE6Nape8Ymb6900UmJI vQ7oh4AEt/d3jCux9RZSWo7iAa4etHwk0N7CdTbpzcyNyEtanMTIYpOCOiU3qB+cAsZf Th474qmbOmeb5pYOM+djFXYW0aEHpaBVokG8xVPlFwfr2/+HcdZqveYc0DLBLzMAowAK IXy72CA1Gl+Wg1JXb0cDZ3At/5k1DkJ/ZiM57tNd6RbnwurWKGg/j07CSYyfV9RgTqAj FesdlluvuTD5LGtIaWEQA1G9lQw7IsvsKWu6ClSMAeFqX5JNTpCSRMoR6HphJbW6FfgX fGVQ==
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=/rspa1hOyfj9paKWlmQeAR1NhOjB1Gg8oQ3LOCn9S6k=; b=V05yJ2yj3m9eMOwXo1WBqWAnBWPgJ2dbDsNh5NUqM2XY097m9zs+wfoOxyzzWKU6y+ luE2M1fskKrJA0HikD/uwurjzY54rnrkoDEf3HZmlbRUi6vUxGGSDL+fhZiDi7SR8D9O VhZXo6OakMZ/GxWJVjBYlFup+VLbPaRU492Eg=
In-reply-to: <20131128234441.GQ10323@xxxxxxxxxxxxxxxxxx>
References: <20131124140413.GA19271@xxxxxxxxxxxxx> <20131124152758.GL10323@xxxxxxxxxxxxxxxxxx> <20131125160648.GA4933@xxxxxxxxxxxxx> <20131126131134.GM10323@xxxxxxxxxxxxxxxxxx> <20131126141253.GA28062@xxxxxxxxxxxxx> <20131127064351.GN10323@xxxxxxxxxxxxxxxxxx> <20131127100906.GA19740@xxxxxxxxxxxxx> <20131128162618.GO10323@xxxxxxxxxxxxxxxxxx> <20131128212301.GP10323@xxxxxxxxxxxxxxxxxx> <20131128225102.GS10988@dastard> <20131128234441.GQ10323@xxxxxxxxxxxxxxxxxx>
Sender: linus971@xxxxxxxxx
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

<Prev in Thread] Current Thread [Next in Thread>