On Thu, 21 Jul 2011, Linus Torvalds wrote:

> Oh, this didn't get marked for stable.
> 
> It's commit 59430262401b, "vfs: fix race in rcu lookup of pruned
> dentry", and it applies to 39 too.
> 
> It turns out 2.6.38 got it right. The bug was introduced by
> 62a7375e5d77 ("vfs - check non-mountpoint dentry might block in
> __follow_mount_rcu()").

Mike Waychison observes that although that patch applies to 39
in principle, it doesn't quite apply in practice, because of the
"reverse_transit" business in 2.6.39's __follow_mount_rcu().

Below is the obvious backport to 2.6.39-stable, which we believe
to be correct; but notice how follow_dotdot_rcu() additionally
calls __follow_mount_rcu() in that release, and it is also
updating inode without the seqcount care which __d_lookup_rcu()
exercised.

I think that's okay, that there's pinning implicit in the dotdot
lookup, which makes it safe.  But please don't take my word for it,
without an ack from Al.

Thanks,
Hugh

Backport of commit 59430262401bec02d415179c43dbe5b8819c09ce
Author: Linus Torvalds <[email protected]>
Date:   Mon Jul 18 15:43:29 2011 -0700

    vfs: fix race in rcu lookup of pruned dentry
    
    Don't update *inode in __follow_mount_rcu() until we'd verified that
    there is mountpoint there.  Kudos to Hugh Dickins for catching that
    one in the first place and eventually figuring out the solution (and
    catching a braino in the earlier version of patch).
    
    Signed-off-by: Linus Torvalds <[email protected]>
    Signed-off-by: Al Viro <[email protected]>

--- 2.6.39.3/fs/namei.c 2011-07-26 12:19:53.898417508 -0700
+++ stable/fs/namei.c   2011-07-26 12:31:26.201850987 -0700
@@ -1013,7 +1013,6 @@ static bool __follow_mount_rcu(struct na
                 * Don't forget we might have a non-mountpoint managed dentry
                 * that wants to block transit.
                 */
-               *inode = path->dentry->d_inode;
                if (!reverse_transit &&
                     unlikely(managed_dentry_might_block(path->dentry)))
                        return false;
@@ -1027,6 +1026,12 @@ static bool __follow_mount_rcu(struct na
                path->mnt = mounted;
                path->dentry = mounted->mnt_root;
                nd->seq = read_seqcount_begin(&path->dentry->d_seq);
+               /*
+                * Update the inode too. We don't need to re-check the
+                * dentry sequence number here after this d_inode read,
+                * because a mount-point is always pinned.
+                */
+               *inode = path->dentry->d_inode;
        }
 
        if (unlikely(path->dentry->d_flags & DCACHE_NEED_AUTOMOUNT))

_______________________________________________
stable mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/stable

Reply via email to